Skip to content

Support source-phase imports in the acorn/terser optimizer pipeline#26967

Open
guybedford wants to merge 1 commit into
emscripten-core:mainfrom
guybedford:acorn-source-phase-imports
Open

Support source-phase imports in the acorn/terser optimizer pipeline#26967
guybedford wants to merge 1 commit into
emscripten-core:mainfrom
guybedford:acorn-source-phase-imports

Conversation

@guybedford
Copy link
Copy Markdown
Collaborator

With -sSOURCE_PHASE_IMPORTS=1, emcc emits import source wasmModule from './foo.wasm' in its JS runtime, but at -O2+ the optimizer crashes because acorn 8.x doesn't yet understand the syntax. Even once acorn parses it, the bundled terser silently drops the source keyword on the mozilla-AST round-trip, which changes runtime semantics (host returns the exports namespace instead of a WebAssembly.Module).

This wires in the acorn-import-phases plugin so acorn parses the syntax, and pulls in matching terser support so the keyword survives printing. Terser side is emscripten-core/terser#1 (the v4.8.0 downstream port of upstream terser PR terser/terser#1682) — the vendored third_party/terser/terser.js here is rebuilt from that branch.

New fixture test/js_optimizer/sourcePhaseImports.js runs two import source declarations through JSDCE and checks the keyword survives; runs via ./test/runner other.test_js_optimizer_sourcePhaseImports.

That terser PR should ideally land first so this can be rebuilt from a merged branch; happy to rebase the bundle afterwards.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add -O2 to the test_esm_source_phase_imports test? Maybe using @parameterized? I assume if you do this without this patch the test would crash?

Comment thread tools/acorn-optimizer.mjs Outdated
@guybedford guybedford force-pushed the acorn-source-phase-imports branch from 2801da8 to 5fed2cc Compare May 16, 2026 01:50
When -sSOURCE_PHASE_IMPORTS=1, emcc emits
```js
import source wasmModule from './foo.wasm';
```
in its JS runtime. At -O2/-O3/-Os/-Oz the emitted JS is run through
tools/acorn-optimizer.mjs which currently fails with a SyntaxError at
parse time because acorn 8.x does not yet understand the source-phase
imports proposal (https://github.com/tc39/proposal-source-phase-imports).

Wire in the acorn-import-phases plugin so acorn can parse the syntax,
and pull in the matching terser support (downstream of emscripten-core/terser#1)
so the round-trip through from_mozilla_ast / to_mozilla_ast preserves
the `phase` keyword on the way back out. Without the terser side,
terser would silently drop the keyword and the host would return the
module's exports namespace instead of a WebAssembly.Module, changing
runtime semantics.

* package.json: add acorn-import-phases dependency.
* tools/acorn-optimizer.mjs: extend acorn with the plugin and use the
  extended parser at the parse site.
* third_party/terser/terser.js: rebuilt from the emscripten-core/terser
  branch with source-phase imports support (PR
  emscripten-core/terser#1, the v5.18.2
  downstream port of upstream terser PR
  terser/terser#1682).
* test/js_optimizer/sourcePhaseImports{,-output}.js: new fixture that
  feeds two `import source` declarations through the JSDCE pass and
  checks the keyword survives.
* test/test_other.py: register the fixture in test_js_optimizer, and
  parametrize test_esm_source_phase_imports across no-args and -O2 to
  exercise the optimizer pipeline.
@guybedford guybedford force-pushed the acorn-source-phase-imports branch from 5fed2cc to f299e76 Compare May 16, 2026 02:26
@guybedford
Copy link
Copy Markdown
Collaborator Author

Could you add -O2 to the test_esm_source_phase_imports test? Maybe using @parameterized? I assume if you do this without this patch the test would crash?

Sure, I've bumped to -O3 which I believe was causing the crash before.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but maybe we want to wait for the terser changes to land, either in our fork or in upstream. Unless you want to land this first for some reason?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants