feat: hoisting and tdz. (#20)

knightedcodemonkey · web-flow · commit 39719a93a9f8 · 2025-12-27T15:04:49.000-06:00
diff --git a/README.md b/README.md
@@ -133,6 +133,7 @@ Behavior notes (defaults in parentheses)
 - `requireSource` (`builtin`): whether `require` comes from Node or `createRequire`.
 - `cjsDefault` (`auto`): bundler-style default interop vs direct `module.exports`.
 - `out`/`inPlace`: write the transformed code to a file; otherwise the function returns the transformed string only.
+- CommonJS → ESM lowering will throw on `with` statements and unshadowed `eval` calls to avoid unsound rewrites.
 
 See [docs/esm-to-cjs.md](docs/esm-to-cjs.md) for deeper notes on live bindings, interop helpers, top-level await behavior, and `import.meta.main` handling. For CommonJS to ESM lowering details, read [docs/cjs-to-esm.md](docs/cjs-to-esm.md).
 
@@ -141,3 +142,4 @@ See [docs/esm-to-cjs.md](docs/esm-to-cjs.md) for deeper notes on live bindings,
 - Remove `@knighted/specifier` and avoid double parsing.
 - Emit source maps and clearer diagnostics for transform choices.
 - Broaden fixtures covering live-binding and top-level await edge cases across Node versions.
+- Benchmark scope analysis choices: compare `periscopic`, `scope-analyzer`, and `eslint-scope` on fixtures and pick the final adapter.
diff --git a/docs/cjs-to-esm.md b/docs/cjs-to-esm.md
@@ -5,6 +5,7 @@
 - Rewrites CommonJS modules to ESM when `target: 'module'` with `transformSyntax` enabled.
 - Assumes Node 22.21+ runtime with native ESM.
 - Skips lowering when `module` or `exports` are shadowed at module scope to avoid mis-compilation.
+- Throws when encountering `with` statements or unshadowed `eval` to avoid unsound rewrites.
 - Deprecated CJS features (`require.extensions`, `module.parent`, legacy folder-as-module resolution) are left as-is.
 
 ## Imports
diff --git a/docs/esm-to-cjs.md b/docs/esm-to-cjs.md
@@ -3,7 +3,7 @@
 ## Scope
 
 - Translates ESM syntax to CommonJS when `target: 'commonjs'` with `transformSyntax` enabled.
-- Assumes Node 20.11+ runtime with `__filename`/`__dirname` shims supplied by the host.
+- Assumes Node 22.21+ runtime with `__filename`/`__dirname` shims supplied by the host (matches package engines).
 - Keeps optional live-binding behavior via `liveBindings` (strict/loose/off).
 - Top-level await (TLA) handling is controlled by `topLevelAwait: 'error' | 'wrap' | 'preserve'` when lowering to CommonJS.
 - Optional import.meta.main gating via `importMetaMain: 'shim' | 'warn' | 'error'`.
diff --git a/docs/hoisting-tdz.md b/docs/hoisting-tdz.md
@@ -0,0 +1,21 @@
+# Hoisting and TDZ in identifier tracking
+
+## Purpose
+
+This library tracks module-scope identifiers so CJS↔ESM lowering can avoid collisions and emit correct exports/imports. We model only hoisting behaviors that affect module-scope resolution and skip cases that either error at runtime or are handled by the module loader.
+
+## What we treat as hoisted
+
+- `function` declarations at module scope: reads before the declaration are counted.
+- `var` declarations at module scope: reads before the declaration are counted (value is `undefined`).
+
+## What we do not hoist
+
+- `let` / `const` / `class`: reads in the temporal dead zone are ignored for hoist accounting (they are runtime errors). See fixtures/tests under `test/fixtures/identifiers/hoisting/tdz.js`.
+- `import` bindings: import hoisting is handled by the module system; we exclude imports from module-scope hoist tracking. See `test/fixtures/identifiers/hoisting/importHoist.js`.
+- Function declarations inside blocks (strict mode): they stay block-scoped and are not treated as module-scope hoists. See `test/fixtures/identifiers/hoisting/functionInBlock.js`.
+
+## Why this scope
+
+- Our goal is collision-free lowering, not simulating all JS runtime hoisting/TDZ behavior.
+- Excluding TDZ and import hoists keeps the identifier table focused on cases that meaningfully affect CJS↔ESM transforms.
diff --git a/oxlint.json b/oxlint.json
@@ -17,7 +17,10 @@
         "no-unused-expressions": "off",
         "no-dupe-class-members": "off",
         "no-useless-rename": "off",
-        "unicorn/no-empty-file": "off"
+        "unicorn/no-empty-file": "off",
+        "no-with": "off",
+        "no-console": "off",
+        "no-eval": "off"
       }
     },
     {
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@knighted/module",
-  "version": "1.0.0-beta.4",
+  "version": "1.0.0-beta.5",
   "description": "Transforms differences between ES modules and CommonJS.",
   "type": "module",
   "main": "dist/module.js",
diff --git a/src/format.ts b/src/format.ts
@@ -515,6 +515,20 @@ const format = async (src: string, ast: ParseResult, opts: FormatterOptions) =>
         }
       }
 
+      if (shouldRaiseEsm && node.type === 'WithStatement') {
+        throw new Error('Cannot transform to ESM: with statements are not supported.')
+      }
+
+      if (
+        shouldRaiseEsm &&
+        node.type === 'CallExpression' &&
+        node.callee.type === 'Identifier' &&
+        node.callee.name === 'eval' &&
+        !shadowedBindings.has('eval')
+      ) {
+        throw new Error('Cannot transform to ESM: eval is not supported.')
+      }
+
       if (
         shouldRaiseEsm &&
         node.type === 'CallExpression' &&
diff --git a/src/utils.ts b/src/utils.ts
@@ -10,13 +10,13 @@ import { scopeNodes } from './utils/scopeNodes.js'
 
 type UpdateSrcLang = Parameters<Specifier['updateSrc']>[1]
 const getLangFromExt = (filename: string): UpdateSrcLang => {
-  const ext = extname(filename)
+  const ext = extname(filename).toLowerCase()
 
-  if (ext.endsWith('.js')) {
+  if (ext === '.js' || ext === '.mjs' || ext === '.cjs') {
     return 'js'
   }
 
-  if (ext.endsWith('.ts')) {
+  if (ext === '.ts' || ext === '.mts' || ext === '.cts') {
     return 'ts'
   }
 
@@ -276,6 +276,14 @@ const collectModuleIdentifiers = async (ast: Node, hoisting: boolean = true) =>
           const isVarDeclarationInGlobalScope =
             identifier.isVarDeclarationInGlobalScope(ancestors)
 
+          const parent = ancestors[ancestors.length - 2]
+          const grandParent = ancestors[ancestors.length - 3]
+          const hoistSafe =
+            parent.type === 'FunctionDeclaration' ||
+            (parent.type === 'VariableDeclarator' &&
+              grandParent?.type === 'VariableDeclaration' &&
+              grandParent.kind === 'var')
+
           if (
             isModuleScope ||
             isClassOrFuncDeclaration ||
@@ -284,7 +292,7 @@ const collectModuleIdentifiers = async (ast: Node, hoisting: boolean = true) =>
             meta.declare.push(node)
 
             // Check for hoisted reads
-            if (hoisting && globalReads.has(name)) {
+            if (hoisting && hoistSafe && globalReads.has(name)) {
               const reads = globalReads.get(name)
 
               if (reads) {
diff --git a/src/utils/identifiers.ts b/src/utils/identifiers.ts
@@ -155,6 +155,14 @@ const collectModuleIdentifiers = async (ast: Node, hoisting: boolean = true) =>
           const isVarDeclarationInGlobalScope =
             identifier.isVarDeclarationInGlobalScope(ancestors)
 
+          const parent = ancestors[ancestors.length - 2]
+          const grandParent = ancestors[ancestors.length - 3]
+          const hoistSafe =
+            parent.type === 'FunctionDeclaration' ||
+            (parent.type === 'VariableDeclarator' &&
+              grandParent?.type === 'VariableDeclaration' &&
+              grandParent.kind === 'var')
+
           if (
             isModuleScope ||
             isClassOrFuncDeclaration ||
@@ -163,7 +171,7 @@ const collectModuleIdentifiers = async (ast: Node, hoisting: boolean = true) =>
             meta.declare.push(node)
 
             // Check for hoisted reads
-            if (hoisting && globalReads.has(name)) {
+            if (hoisting && hoistSafe && globalReads.has(name)) {
               const reads = globalReads.get(name)
 
               if (reads) {
diff --git a/src/utils/lang.ts b/src/utils/lang.ts
@@ -4,13 +4,13 @@ import type { Specifier } from '@knighted/specifier'
 // Determine language from filename extension for specifier rewrite.
 type UpdateSrcLang = Parameters<Specifier['updateSrc']>[1]
 const getLangFromExt = (filename: string): UpdateSrcLang => {
-  const ext = extname(filename)
+  const ext = extname(filename).toLowerCase()
 
-  if (ext.endsWith('.js')) {
+  if (ext === '.js' || ext === '.mjs' || ext === '.cjs') {
     return 'js'
   }
 
-  if (ext.endsWith('.ts')) {
+  if (ext === '.ts' || ext === '.mts' || ext === '.cts') {
     return 'ts'
   }
 
diff --git a/test/fixtures/computedReexport.cjs b/test/fixtures/computedReexport.cjs
@@ -0,0 +1,8 @@
+exports['foo-bar'] = 'fb'
+exports['zap'] = 2
+
+module.exports = {
+  bag: exports,
+  fooBar: exports['foo-bar'],
+  zap: exports['zap'],
+}
diff --git a/test/fixtures/identifiers/hoisting/functionInBlock.js b/test/fixtures/identifiers/hoisting/functionInBlock.js
@@ -0,0 +1,11 @@
+// In strict mode, function declarations inside blocks are block-scoped and not hoisted
+// to the outer scope. Ensure we don't count reads before the declaration as hoisted.
+
+{
+  func()
+  function func() {
+    return 'block-func'
+  }
+}
+
+export {}
diff --git a/test/fixtures/identifiers/hoisting/importHoist.js b/test/fixtures/identifiers/hoisting/importHoist.js
@@ -0,0 +1,9 @@
+// Imports are hoisted by the module system, but we do not treat them as part of
+// the module-scope identifier hoisting for collision/reads accounting.
+
+import { x } from './importHoistDep.js'
+
+const a = x
+const b = (() => x)()
+
+export { a, b }
diff --git a/test/fixtures/identifiers/hoisting/importHoistDep.js b/test/fixtures/identifiers/hoisting/importHoistDep.js
@@ -0,0 +1 @@
+export const x = 'import-hoist'
diff --git a/test/fixtures/identifiers/hoisting/tdz.js b/test/fixtures/identifiers/hoisting/tdz.js
@@ -0,0 +1,46 @@
+// These TDZ reads would throw at runtime; we should not count them as safe hoists.
+// Access before declaration (let/const/class) remains in the TDZ.
+
+// let
+const a = (() => {
+  try {
+    return foo
+  } catch {
+    return 'tdz'
+  }
+})()
+let foo = 'foo'
+
+// const
+const b = (() => {
+  try {
+    return bar
+  } catch {
+    return 'tdz'
+  }
+})()
+const bar = 'bar'
+
+// class
+const c = (() => {
+  try {
+    return Baz
+  } catch {
+    return 'tdz'
+  }
+})()
+class Baz {}
+
+// nested block TDZ
+{
+  const d = (() => {
+    try {
+      return inner
+    } catch {
+      return 'tdz'
+    }
+  })()
+  const inner = 'inner'
+}
+
+export { a, b, c }
diff --git a/test/fixtures/liveBindingMutation.cjs b/test/fixtures/liveBindingMutation.cjs
@@ -0,0 +1,9 @@
+exports.counter = 0
+
+function inc() {
+  exports.counter += 1
+}
+
+inc()
+
+exports.inc = inc
diff --git a/test/fixtures/nestedShadowExports.cjs b/test/fixtures/nestedShadowExports.cjs
@@ -0,0 +1,8 @@
+exports.foo = 'outer'
+
+function run() {
+  const innerExports = { foo: 'inner' }
+  return innerExports.foo
+}
+
+module.exports = { foo: exports.foo, run }
diff --git a/test/fixtures/withEval.cjs b/test/fixtures/withEval.cjs
@@ -0,0 +1,7 @@
+const obj = { foo: 'bar' }
+
+with (obj) {
+  console.log(foo)
+}
+
+eval('exports.evalled = true')
diff --git a/test/module.ts b/test/module.ts
@@ -251,6 +251,15 @@ describe('@knighted/module', () => {
     assert.equal((mod as any).default.commonjs, true)
   })
 
+  it('throws when encountering with/eval while lowering to esm', async () => {
+    const fixturePath = join(fixtures, 'withEval.cjs')
+
+    await assert.rejects(
+      () => transform(fixturePath, { target: 'module' }),
+      /with statements are not supported|eval is not supported/i,
+    )
+  })
+
   it('keeps nested requires via createRequire when lowering to esm', async t => {
     const fixturePath = join(fixtures, 'nestedRequire.cjs')
     const outFile = join(fixtures, 'nestedRequire.mjs')
@@ -273,6 +282,26 @@ describe('@knighted/module', () => {
     assert.equal((mod as any).default.commonjs, true)
   })
 
+  it('preserves outer exports when inner scopes shadow exports', async t => {
+    const fixturePath = join(fixtures, 'nestedShadowExports.cjs')
+    const outFile = join(fixtures, 'nestedShadowExports.mjs')
+
+    t.after(() => {
+      rm(outFile, { force: true })
+    })
+
+    const result = await transform(fixturePath, { target: 'module' })
+    await writeFile(outFile, result)
+
+    const { status } = spawnSync('node', [outFile], { stdio: 'inherit' })
+    assert.equal(status, 0)
+
+    const mod = await import(pathToFileURL(outFile).href)
+    assert.equal((mod as any).default.foo, 'outer')
+    assert.equal(typeof (mod as any).default.run, 'function')
+    assert.equal((mod as any).default.run(), 'inner')
+  })
+
   it('keeps block-scoped require via createRequire when lowering to esm', async t => {
     const fixturePath = join(fixtures, 'blockRequire.cjs')
     const outFile = join(fixtures, 'blockRequire.mjs')
@@ -314,6 +343,46 @@ describe('@knighted/module', () => {
     assert.equal((mod as any).baz, 3)
   })
 
+  it('preserves runtime mutations on the exports bag default', async t => {
+    const fixturePath = join(fixtures, 'liveBindingMutation.cjs')
+    const outFile = join(fixtures, 'liveBindingMutation.mjs')
+
+    t.after(() => {
+      rm(outFile, { force: true })
+    })
+
+    const result = await transform(fixturePath, { target: 'module' })
+    await writeFile(outFile, result)
+
+    const { status } = spawnSync('node', [outFile], { stdio: 'inherit' })
+    assert.equal(status, 0)
+
+    const mod = await import(pathToFileURL(outFile).href)
+    assert.equal((mod as any).counter, 1)
+    ;(mod as any).inc()
+    assert.equal(typeof (mod as any).inc, 'function')
+  })
+
+  it('exports computed property names when lowering to esm', async t => {
+    const fixturePath = join(fixtures, 'computedReexport.cjs')
+    const outFile = join(fixtures, 'computedReexport.mjs')
+
+    t.after(() => {
+      rm(outFile, { force: true })
+    })
+
+    const result = await transform(fixturePath, { target: 'module' })
+    await writeFile(outFile, result)
+
+    const { status } = spawnSync('node', [outFile], { stdio: 'inherit' })
+    assert.equal(status, 0)
+
+    const mod = await import(pathToFileURL(outFile).href)
+    assert.equal((mod as any).default.fooBar, 'fb')
+    assert.equal((mod as any).default.zap, 2)
+    assert.equal((mod as any).default.bag['foo-bar'], 'fb')
+  })
+
   it('rewrites require.main to import.meta.main', async t => {
     const fixturePath = join(fixtures, 'requireMain.cjs')
     const outFile = join(fixtures, 'requireMain.mjs')
diff --git a/test/utils.ts b/test/utils.ts

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,10 @@`
`17`	`17`	`"no-unused-expressions": "off",`
`18`	`18`	`"no-dupe-class-members": "off",`
`19`	`19`	`"no-useless-rename": "off",`
`20`		`- "unicorn/no-empty-file": "off"`
	`20`	`+ "unicorn/no-empty-file": "off",`
	`21`	`+ "no-with": "off",`
	`22`	`+ "no-console": "off",`
	`23`	`+ "no-eval": "off"`
`21`	`24`	`}`
`22`	`25`	`},`
`23`	`26`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@knighted/module",`
`3`		`- "version": "1.0.0-beta.4",`
	`3`	`+ "version": "1.0.0-beta.5",`
`4`	`4`	`"description": "Transforms differences between ES modules and CommonJS.",`
`5`	`5`	`"type": "module",`
`6`	`6`	`"main": "dist/module.js",`