Skip to content

Commit ea66111

Browse files
robertsLandoclaude
andauthored
fix(detector): pick up six more silent-drop shapes (re-exports, new URL, multi-arg path.join, path.resolve(__dirname,…), createRequire aliases, import.meta.resolve) (#272)
* fix(detector): pick up ESM re-exports, new URL/import.meta.url, multi-arg path.join, path.resolve(__dirname,...), createRequire aliases, import.meta.resolve Follow-up to #268. Six more detector shapes that silently dropped their target — most visible in SEA mode where the ESM→CJS transform is skipped and the detector is the only dependency pass. - ESM re-exports (`export * from "lit"` / `export { x } from "lit"` / `export * as ns from "lit"`). visitorImport only matched ImportDeclaration; ExportAllDeclaration / ExportNamedDeclaration with a `.source` now go through a parallel visitorReExport. - `new URL("./rel", import.meta.url)` — canonical ESM sibling-asset idiom. Treated as ALIAS_AS_RELATIVE; only matches when the base is literally `import.meta.url` so we don't synthesize paths from arbitrary URL bases. - multi-arg `path.join(__dirname, "a", "b", …)` and `path.resolve(__dirname, "a", "b", …)`. Old code required exactly 2 args; now any literal-only segment list joins via path.posix.join to a single posix alias. Bails on non-literal segments to avoid guessing. - `path.resolve(__dirname, "lit")`. Same intent as `path.join`; also silences the spurious "ambiguous resolve" warning that visitorUseSCWD used to emit for this exact shape (the call is now bundled, so the warning would contradict our own action). - `createRequire(import.meta.url)("./foo")` and the aliased form `const r = createRequire(...); r("./foo")`. detect() now pre-scans the AST for VariableDeclarator/AssignmentExpression bindings whose RHS is a `createRequire(...)` call, threads the bound names through the visitor as a 3rd arg, and visitorRequire / visitorRequireResolve treat them as require-equivalent. - `import.meta.resolve("lit")` — modern ESM resolver API. Same ALIAS_AS_RESOLVABLE handling as `require.resolve`. The issue's claim about `require.resolve("lit", { paths: [...] })` silently dropping was verified to be wrong — `valid2(null)` already accepts the ObjectExpression-as-2nd-arg case, so existing code bundles the target correctly. No change there. Walker.ts forwards the new `requireAliases` Set from detect()'s visitor signature into visitorSuccessful so per-file alias state is applied. Closes #269 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(detector): address Copilot review on #272 - visitorNewURL & visitorImportMetaResolve: tighten the MetaProperty guard. `MetaProperty` also represents `new.target`, so checking for the type alone matched `new URL("./x", new.target.url)` and a hypothetical `new.target.resolve(...)`. Added isImportMeta() that asserts the meta/property pair is exactly `import`/`meta`. - collectRequireAliases: drop the deep traversal in favor of a single pass over `program.body` and only accept top-level `const` bindings. Pre-fix the rule was scope-blind and would shadow-leak `r` from an inner function into the outer scope. Top-level-`const`-only covers the canonical idiom (`const r = createRequire(import.meta.url)` at module top) without needing real scope tracking. - visitorUseSCWD: only skip the warning when *every* arg after __dirname is a literal — i.e. when visitorPathJoin actually claims the call. Dynamic shapes like `path.resolve(__dirname, x)` now warn again, since visitorPathJoin bails on the non-literal segment and the diagnostic is the only signal the user gets. - visitorNonLiteral / visitorMalformed (and their helpers): thread `requireAliases` so aliased dynamic / malformed requires (`r(x)` / `r()`) emit the same warnings as the literal `require` forms instead of being silently dropped. Five new unit tests cover the tightened behaviors; verified each fails without this commit and passes with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(detector): reject scheme-prefixed new URL() first args new URL("https://…"|"data:…"|"file:…", import.meta.url) is an absolute URL whose base is ignored, so it never resolves to a snapshot-relative asset. visitorNewURL still matched it, bundling a bogus path and emitting a spurious "Cannot stat" warning at top level. Guard against any scheme-prefixed literal; scheme-less relative/bare sibling names still match. Adds regression tests for the absolute/data/file URL cases and the bare-name case. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1b0002f commit ea66111

3 files changed

Lines changed: 660 additions & 38 deletions

File tree

0 commit comments

Comments
 (0)