Skip to content

detector: silent dependency drops for several ESM/Node shapes #269

@robertsLando

Description

@robertsLando

Follow-up from #264 / #268. While auditing lib/detector.ts I found additional call-site shapes the walker silently drops — same class of bug as the dynamic-import miss fixed in #268 (no warning, dependency simply not bundled, workaround is pkg.scripts). These matter most in SEA mode, where the ESM→CJS transform is skipped and the detector is the only dependency pass.

Bugs (common patterns, silent drops)

  • re-exportsexport * from "lit" / export { x } from "lit"
    visitorImport only matches ImportDeclaration. Babel exposes re-exports as ExportAllDeclaration / ExportNamedDeclaration with a .source. lib/esm-transformer.ts:311-317 handles them for the ESM→CJS transform, but that transform is skipped in SEA mode — so every ESM barrel file drops its re-exports.

  • new URL("./rel", import.meta.url)
    Canonical ESM idiom for sibling assets (the ESM equivalent of path.join(__dirname, …)). Not matched anywhere, so data files loaded this way never make it into the snapshot.

  • multi-arg path.join(__dirname, "a", "b", …)
    visitorPathJoin at lib/detector.ts:231 requires exactly 2 args (n.arguments.length === 2). The // TODO concat them comment is stale. Any 3+ segment join is silently skipped.

  • require.resolve("lit", { paths: [...] })
    The options-object form is idiomatic on modern Node. valid2 only accepts strings / pkg's exclusion markers, so these calls fall through and the target is never bundled.

Enhancements

  • path.resolve(__dirname, "lit")
    Same intent as path.join(__dirname, "lit") and widely used. Today visitorUseSCWD warns on path.resolve(...) regardless of whether __dirname is the first arg — so we both miss the bundle and emit a spurious "ambiguous" warning.

  • createRequire(import.meta.url) bound to a non-require name
    const r = createRequire(…); r('./foo') is invisible because visitorRequire gates on callee.name === 'require'. Direct createRequire(…)('./foo') and reassignments to a require-named local work today; anything else doesn't.

  • import.meta.resolve("lit")
    Modern ESM resolver API, gradually replacing require.resolve in ESM code. Not matched.

Out of scope

new Worker(url), child_process.fork(path), fs.readFileSync('./literal') — handled elsewhere in pkg via scripts/assets config and the worker bootstrap. Rolling them into the detector would overlap with existing machinery.

Suggested ordering

Re-exports and options-object require.resolve are one-liner additions and the highest-impact (barrel files and options-object require.resolve are both very common in modern codebases). new URL(...) and path.resolve(__dirname, ...) are the ESM equivalents of existing CJS detectors and belong together. Multi-arg path.join is a two-liner. createRequire aliasing and import.meta.resolve can follow.

Happy to take a pass at this — opening for visibility and to avoid conflating with #268.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions