Skip to content

removeDynamicImportsPlugin silently stubs dynamic imports in third-party code #978

Description

@rekmarks

Note on this issue number: this was previously "Consider caching vat bundles". That content has been folded into #991, which covers bundle identity and caching together as one content-addressed store. This number has been reused for a correctness concern that surfaced while investigating it.

removeDynamicImportsPlugin (packages/kernel-utils/src/vite-plugins/bundle-vat.ts:33-60) rewrites every import('literal') in the bundle graph to Promise.resolve({}).

The reason is legitimate, and the plugin's own comment explains it: rolldown refuses IIFE output when any module in the graph contains a dynamic import, even with output.codeSplitting disabled. Some dependencies use lazy await import(x) in utility functions — viem is named — that vats never actually call.

The concern is the failure mode when that assumption is wrong.

The transform applies across the whole dependency graph, including third-party code we do not control, and it does not fail — it substitutes an empty object. So if a vat ever does reach one of those code paths:

  • the dynamic import resolves successfully to {}
  • every property access on the result is undefined
  • the failure surfaces somewhere unrelated, as a TypeError or as silently wrong behaviour

That is a silent wrong answer rather than an error, in code we rewrote without the dependency author's knowledge. It also means a dependency version bump can quietly add a newly-reachable dynamic import, and nothing tells us.

Secondary effect: two parties building the same vat from different transitive dependency versions can get different bundles with no signal — relevant to #991, since bundle identity is only meaningful if the build is reproducible.

Options, roughly in increasing cost:

  1. Fail loudly instead of silently. Replace Promise.resolve({}) with a rejected promise, or a thunk that throws a clear "dynamic import was stripped at bundle time" error. Same build outcome, but reachable paths become visible immediately instead of turning into undefined. Cheapest real improvement and probably worth doing regardless of what else we choose.
  2. Report what was stripped. Have the plugin emit the list of modules and specifiers it rewrote, so the assumption is auditable and reviewable rather than invisible.
  3. Scope the transform. Only rewrite in node_modules, or only for an explicit allowlist of specifiers we have checked, rather than the entire graph by default.
  4. Avoid the constraint. Emit a format that tolerates dynamic imports, or preserve the module graph. See Add a content-addressed vat bundle store (identity, verification, caching) #991 for why moving to endoZipBase64 is not a small change — but it is the one thing that would remove the need for this plugin entirely.

Option 1 alone would convert this from a latent silent failure into a loud one, which is most of the value.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions