fix(detector): stop silent dependency drops in SEA mode (ESM parse, dynamic import, decorators) (#268)

* fix(sea): parse ESM files as modules and walk dynamic import() literals

`detector.parse()` called babel with the default `sourceType: 'script'`,
so SEA-mode walker runs over `import.meta` / top-level `await` failed
to parse and silently skipped the file's dependency traversal. Thread
`isESMFile(record.file)` through `stepDetect` → `detect()` → `parse()`
so ESM files get `sourceType: 'module'`. Also teach the visitor to
recognize `import('literal')` `CallExpression`s so bundler-emitted
dynamic imports are bundled like static ones.

* docs(detector): add concise jsdoc for visitor and helper functions

* fix(detector): enable decorators-legacy plugin for babel parse

Third-party sources that ship raw `@decorator` syntax (fontkit, older
MobX / Nest builds) tripped the same silent-drop failure mode as #264:
babel.parse threw, `detect()` logged a warning, and the file's
dependency graph was dropped. Enable `decorators-legacy` in both the
walker's detector and the ESM-transformer parse calls so these sources
parse cleanly and their requires/imports get bundled. Extend test-94
with a decorator fixture walked via `pkg.scripts`.

* fix(detector): reject non-string specifiers in dynamic import matcher

Guard visitorDynamicImport against `import(0)` / `import(true)` so a
numeric or boolean literal can't flow through the walker as an alias and
crash downstream string checks (e.g. isBuiltin's moduleName.startsWith).

Addresses Copilot review feedback on PR #268.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

Author: robertsLando

eslint.config.js

@@ -15,6 +15,8 @@ module.exports = [
       'test/test-51-esm-import-meta/esm-module/test-import-meta-basic.js',
       'lib/log.js', // ESM re-export file
       'test/test-50-extensions/test-y-esnext.js', // ESM test file
+      'test/test-94-sea-esm-import-meta/app/lib/decorated.js', // decorator syntax fixture
+
       'prelude/sea-bootstrap.bundle.js', // Generated by esbuild
       'prelude/sea-bootstrap-esm.bundle.mjs', // Generated by esbuild
     ],

lib/detector.ts

@@ -5,6 +5,7 @@ import { log } from './log';
 
 import { ALIAS_AS_RELATIVE, ALIAS_AS_RESOLVABLE } from './common';
 
+/** Type guard for plain literal nodes; rejects template literals with interpolations. */
 function isLiteral(node: babelTypes.Node): node is babelTypes.Literal {
   if (node == null) {
     return false;
@@ -21,6 +22,7 @@ function isLiteral(node: babelTypes.Node): node is babelTypes.Literal {
   return true;
 }
 
+/** Extracts the runtime value of a literal. Throws on null/regexp — never valid module specifiers. */
 function getLiteralValue(node: babelTypes.Literal) {
   if (node.type === 'TemplateLiteral') {
     return node.quasis[0].value.raw;
@@ -37,6 +39,7 @@ function getLiteralValue(node: babelTypes.Literal) {
   return node.value;
 }
 
+/** Renders an import specifier list back to source (`a, { b, c as d }`) for log output. */
 function reconstructSpecifiers(
   specs: (
     | babelTypes.ImportDefaultSpecifier
@@ -79,6 +82,7 @@ function reconstructSpecifiers(
   return defaults.join(', ');
 }
 
+/** Prints any AST node back to a single-line source string, used when an arg isn't a literal. */
 function reconstruct(node: babelTypes.Node) {
   let v = generate(node, { comments: false }).code.replace(/\n/g, '');
   let v2;
@@ -102,6 +106,7 @@ interface Was {
   v3?: string;
 }
 
+/** Fills a template (e.g. `require({v1}{c2}{v2})`) with captured args to produce the printable form of a match. */
 function forge(pattern: string, was: Was) {
   return pattern
     .replace('{c1}', ', ')
@@ -112,6 +117,7 @@ function forge(pattern: string, was: Was) {
     .replace('{v3}', was.v3 ? was.v3 : '');
 }
 
+/** Guards the 2nd arg of require/require.resolve — only pkg's `must-exclude`/`may-exclude` markers are honored. */
 function valid2(v2?: Was['v2']) {
   return (
     v2 === undefined ||
@@ -121,6 +127,7 @@ function valid2(v2?: Was['v2']) {
   );
 }
 
+/** Matches `require.resolve("lit"[, "lit"])`. Returns captured args or null. */
 function visitorRequireResolve(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -150,6 +157,7 @@ function visitorRequireResolve(n: babelTypes.Node) {
   };
 }
 
+/** Matches `require("lit"[, "lit"])`. Returns captured args or null. */
 function visitorRequire(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -173,6 +181,7 @@ function visitorRequire(n: babelTypes.Node) {
   };
 }
 
+/** Matches a static ESM `import … from "lit"` declaration. */
 function visitorImport(n: babelTypes.Node) {
   if (!babelTypes.isImportDeclaration(n)) {
     return null;
@@ -181,6 +190,32 @@ function visitorImport(n: babelTypes.Node) {
   return { v1: n.source.value, v3: reconstructSpecifiers(n.specifiers) };
 }
 
+/** Matches dynamic `import("lit")` so bundler-emitted chunk splits get walked like static imports. */
+function visitorDynamicImport(n: babelTypes.Node) {
+  if (!babelTypes.isCallExpression(n)) {
+    return null;
+  }
+
+  if (n.callee.type !== 'Import') {
+    return null;
+  }
+
+  if (!n.arguments || !isLiteral(n.arguments[0])) {
+    return null;
+  }
+
+  // Module specifiers are always strings — reject `import(0)` / `import(true)`
+  // so a non-string value can't reach the walker's alias handling.
+  const value = getLiteralValue(n.arguments[0] as babelTypes.Literal);
+
+  if (typeof value !== 'string') {
+    return null;
+  }
+
+  return { v1: value };
+}
+
+/** Matches `path.join(__dirname, "lit")` — treats the joined path as a snapshot asset reference. */
 function visitorPathJoin(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -221,6 +256,11 @@ function visitorPathJoin(n: babelTypes.Node) {
   return { v1: getLiteralValue(n.arguments[1] as babelTypes.StringLiteral) };
 }
 
+/**
+ * Runs each literal-arg matcher in order and returns the first hit as a
+ * `{alias, aliasType, mustExclude?, mayExclude?}` derivative for the walker to
+ * bundle. When `test` is true returns a printable form (used by unit tests).
+ */
 export function visitorSuccessful(node: babelTypes.Node, test = false) {
   let was: Was | null = visitorRequireResolve(node);
 
@@ -270,6 +310,16 @@ export function visitorSuccessful(node: babelTypes.Node, test = false) {
     return { alias: was.v1, aliasType: ALIAS_AS_RESOLVABLE };
   }
 
+  was = visitorDynamicImport(node);
+
+  if (was) {
+    if (test) {
+      return forge('import({v1})', was);
+    }
+
+    return { alias: was.v1, aliasType: ALIAS_AS_RESOLVABLE };
+  }
+
   was = visitorPathJoin(node);
 
   if (was) {
@@ -283,6 +333,7 @@ export function visitorSuccessful(node: babelTypes.Node, test = false) {
   return null;
 }
 
+/** Matches `require.resolve(<non-literal>[, "lit"])` — feeds the "Cannot resolve" warning path. */
 function nonLiteralRequireResolve(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -322,6 +373,7 @@ function nonLiteralRequireResolve(n: babelTypes.Node) {
   };
 }
 
+/** Matches `require(<non-literal>[, "lit"])` — feeds the "Cannot resolve" warning path. */
 function nonLiteralRequire(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -355,6 +407,7 @@ function nonLiteralRequire(n: babelTypes.Node) {
   };
 }
 
+/** Entry visitor for dynamic requires whose target isn't known at build time — returns the alias to warn about. */
 export function visitorNonLiteral(n: babelTypes.Node) {
   const was = nonLiteralRequireResolve(n) || nonLiteralRequire(n);
 
@@ -373,6 +426,7 @@ export function visitorNonLiteral(n: babelTypes.Node) {
   return null;
 }
 
+/** Loose `require(...)` match (no literal gate) — used only to surface malformed-require diagnostics. */
 function isRequire(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -395,6 +449,7 @@ function isRequire(n: babelTypes.Node) {
   return { v1: reconstruct(n.arguments[0]) };
 }
 
+/** Loose `require.resolve(...)` match (no literal gate) — used only for malformed-require diagnostics. */
 function isRequireResolve(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -423,6 +478,7 @@ function isRequireResolve(n: babelTypes.Node) {
   return { v1: reconstruct(n.arguments[0]) };
 }
 
+/** Fires on require/require.resolve shapes the literal matchers rejected (wrong arg count, etc.). */
 export function visitorMalformed(n: babelTypes.Node) {
   const was = isRequireResolve(n) || isRequire(n);
 
@@ -433,6 +489,7 @@ export function visitorMalformed(n: babelTypes.Node) {
   return null;
 }
 
+/** Flags `path.resolve(...)` so the walker can warn that it resolves against `process.cwd()` at runtime, not `__dirname`. */
 export function visitorUseSCWD(n: babelTypes.Node) {
   if (!babelTypes.isCallExpression(n)) {
     return null;
@@ -463,6 +520,11 @@ export function visitorUseSCWD(n: babelTypes.Node) {
 
 type VisitorFunction = (node: babelTypes.Node, trying?: boolean) => boolean;
 
+/**
+ * Iterative DFS over the AST. `visitor` returns true to descend into children;
+ * `trying` is propagated inside try/catch bodies so the walker can downgrade
+ * downstream warnings to debug.
+ */
 function traverse(ast: babelTypes.File, visitor: VisitorFunction) {
   // modified esprima-walk to support
   // visitor return value and "trying" flag
@@ -495,18 +557,37 @@ function traverse(ast: babelTypes.File, visitor: VisitorFunction) {
   }
 }
 
-export function parse(body: string) {
+/**
+ * `babel.parse` wrapper. `isEsm` selects `sourceType: 'module'` so `import.meta`
+ * / top-level await parse cleanly. `decorators-legacy` is enabled so third-party
+ * sources that ship raw `@decorator` syntax (fontkit, older MobX/Nest builds,
+ * etc.) don't trip the parser and silently drop their dependency graph.
+ */
+export function parse(body: string, isEsm = false) {
   return babel.parse(body, {
     allowImportExportEverywhere: true,
     allowReturnOutsideFunction: true,
+    sourceType: isEsm ? 'module' : 'script',
+    plugins: ['decorators-legacy'],
   });
 }
 
-export function detect(body: string, visitor: VisitorFunction, file?: string) {
+/**
+ * Parses `body` and walks the AST with `visitor`. Parse failures are logged
+ * (not thrown) so one unparseable file doesn't abort the whole build — but the
+ * file's dependencies are then skipped, which is why callers must pass the
+ * correct `isEsm` flag.
+ */
+export function detect(
+  body: string,
+  visitor: VisitorFunction,
+  file?: string,
+  isEsm = false,
+) {
   let json;
 
   try {
-    json = parse(body);
+    json = parse(body, isEsm);
   } catch (error) {
     const fileInfo = file ? ` in ${file}` : '';
     log.warn(`Babel parse has failed: ${(error as Error).message}${fileInfo}`);

lib/esm-transformer.ts

@@ -35,7 +35,7 @@ function hasImportMeta(code: string): boolean {
   try {
     const ast = babel.parse(code, {
       sourceType: 'module',
-      plugins: [],
+      plugins: ['decorators-legacy'],
     });
 
     if (!ast) {
@@ -85,7 +85,7 @@ function detectESMFeatures(
   try {
     const ast = babel.parse(code, {
       sourceType: 'module',
-      plugins: [],
+      plugins: ['decorators-legacy'],
     });
 
     if (!ast) {
@@ -300,7 +300,7 @@ export function transformESMtoCJS(
       // Parse the code to check for exports and collect imports
       const ast = babel.parse(code, {
         sourceType: 'module',
-        plugins: [],
+        plugins: ['decorators-legacy'],
       });
 
       let hasExports = false;

lib/walker.ts

@@ -325,6 +325,7 @@ function stepDetect(
         return true; // can i go inside?
       },
       record.file,
+      isESMFile(record.file),
     );
   } catch (error) {
     log.error((error as Error).message, record.file);

test/test-94-sea-esm-import-meta/app/index.mjs

@@ -0,0 +1,12 @@
+// `import.meta` is only valid in `sourceType: "module"`. Before the fix for
+// issue #264 the SEA walker parsed this body in script mode, so the parse
+// failed and the detector never saw the imports below — neither the static
+// one nor the dynamic one ended up in the snapshot.
+import { greet } from './lib/helper.mjs';
+
+const here = new URL(import.meta.url).pathname;
+console.log('here:' + here.split('/').pop());
+console.log('static:' + greet('world'));
+
+const dyn = await import('./lib/dyn.mjs');
+console.log('dynamic:' + dyn.shout('world'));

test/test-94-sea-esm-import-meta/app/lib/decorated.js

@@ -0,0 +1,16 @@
+// Decorator syntax forces the walker's Babel parser to enable the
+// `decorators-legacy` plugin. Without it pkg would log "Babel parse has
+// failed: This experimental syntax requires enabling one of the following
+// parser plugin(s)" and silently drop this file's dependency graph. This
+// file is walked via `pkg.scripts` but never imported at runtime — Node
+// can't execute raw decorator syntax.
+function log(Cls) {
+  return Cls;
+}
+
+@log
+export class Widget {
+  greet(name) {
+    return 'widget:' + name;
+  }
+}

test/test-94-sea-esm-import-meta/app/lib/dyn.mjs

@@ -0,0 +1,3 @@
+export function shout(name) {
+  return 'HELLO ' + name.toUpperCase();
+}

test/test-94-sea-esm-import-meta/app/lib/helper.mjs

@@ -0,0 +1,3 @@
+export function greet(name) {
+  return 'hello ' + name;
+}

test/test-94-sea-esm-import-meta/app/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "test-94-sea-esm-import-meta",
+  "version": "1.0.0",
+  "type": "module",
+  "main": "index.mjs",
+  "bin": "index.mjs",
+  "pkg": {
+    "scripts": [
+      "lib/decorated.js"
+    ]
+  }
+}

test/test-94-sea-esm-import-meta/main.js

@@ -0,0 +1,52 @@
+#!/usr/bin/env node
+
+'use strict';
+
+const assert = require('assert');
+const utils = require('../utils.js');
+
+// Enhanced SEA requires Node.js >= 22
+if (utils.getNodeMajorVersion() < 22) {
+  return;
+}
+
+assert(__dirname === process.cwd());
+
+const input = './app/package.json';
+const testName = 'test-94-sea-esm-import-meta';
+
+const SEA_PLATFORM_SUFFIX = {
+  linux: 'linux',
+  darwin: 'macos',
+  win32: 'win.exe',
+};
+const suffix = SEA_PLATFORM_SUFFIX[process.platform];
+
+const newcomers = utils.seaHostOutputs(testName);
+const before = utils.filesBefore(newcomers);
+
+// Capture pkg's output so we can assert the Babel parse warning (issue #264)
+// never surfaces when walking ESM files that use `import.meta`.
+const args = suffix
+  ? [input, '--sea', '--target', 'host', '--output', `${testName}-${suffix}`]
+  : [input, '--sea'];
+
+const build = utils.pkg.sync(args, { stdio: ['pipe', 'pipe', 'pipe'] });
+const buildLog = build.stdout + build.stderr;
+
+assert(
+  buildLog.indexOf('Babel parse has failed') === -1,
+  'pkg must parse ESM files as modules (issue #264)\npkg output was:\n' +
+    buildLog,
+);
+
+// A successful parse means both imports were walked and bundled — running the
+// binary proves it by printing the imported values. Skip on unsupported hosts.
+if (suffix) {
+  utils.assertSeaOutput(
+    testName,
+    'here:index.mjs\nstatic:hello world\ndynamic:HELLO WORLD\n',
+  );
+}
+
+utils.filesAfter(before, newcomers, { tolerateWindowsEbusy: true });