Support ESM export names that are not valid JS identifiers#27236
Open
guybedford wants to merge 12 commits into
Open
Support ESM export names that are not valid JS identifiers#27236guybedford wants to merge 12 commits into
guybedford wants to merge 12 commits into
Conversation
sbc100
reviewed
Jul 2, 2026
sbc100
left a comment
sbc100
left a comment
Collaborator
There was a problem hiding this comment.
How does this relate to the existing js_manipulation.isidentifier that generates the export name is not a valid JS symbol warning?
i.e. what happens if I do -sEXPORTED_RUNTIME_METHODS=delete in a non-ESM build? I assume we should be disallowing keywords like delete in js_manipulation.isidentifier?
Collaborator
Author
It complements it - that is only called for
It is now supported and tested in this PR.
As above, |
Building with -sWASM_ESM_INTEGRATION at -O2 or above failed in the JS
optimizer's metadce pass with 'could not find the assignment to
"wasmImports"'. In this mode the wasm<->JS boundary is expressed with
native ES import/export syntax rather than the wasmImports object and
wasmExports['x'] member uses that metadce pattern-matches.
Teach the three metadce-related acorn-optimizer passes about the ES form:
- emitDCEGraph reads 'import {..} from "./x.wasm"' as wasm export nodes
(collapsing aliases such as memory/wasmMemory to one node) and
'export { js as wasmName }' as wasm import edges, leaving re-exports
(export { _main }) to root naturally.
- applyDCEGraphRemovals prunes unused specifiers from those statements.
- applyImportAndExportNameChanges applies minified names to the
wasm-facing side of each specifier.
building.py also drops dropped exports (including internal ones like the
indirect function table) from the JS import to keep the two interfaces
in sync, and link.py keeps import/export name minification on for ESM
while disabling the now-pointless import module name minification.
# Conflicts: # tools/building.py
Under SOURCE_PHASE_IMPORTS the glue emits `import source wasmModule from './x.wasm'`, whose lone specifier is an ImportDefaultSpecifier with no `imported`. isImportFromWasm matched it (source ends in .wasm) and emitDCEGraph then threw reading `spec.imported.name`. Gate isImportFromWasm on `!node.phase` so only value-phase wasm imports (the wasm export bindings) are treated as export edges.
Export names that are JS reserved words (e.g. `default`) or otherwise not
valid binding identifiers previously produced broken output such as
`var default = ...` / `export { default }`.
Such names are now bound to a legal identifier internally (memoized, with a
numeric suffix to disambiguate collisions) and exported under their original
name via an alias, e.g. `var $default = ...; export { $default as default }`.
Combined with SELF_INIT (which frees the `default` export slot) this allows a
program to expose its own `default` export.
Under MODULARIZE=instance the glue emits `export { \$default as default }`
for a reserved-word runtime export. emitDCEGraph classified any aliased
`export { x as y }` as a JS function exported to wasm (a wasm import edge),
so `default` was recorded as a wasm import; nothing in wasm uses it, so it
landed in unusedImports and applyDCEGraphRemovals dropped the specifier,
leaving a bare `export` that fails to parse in the next pass.
A wasm import name is a C symbol and can never be `default`, so exclude
`default` from the import-edge classification in emitDCEGraph and from the
specifier removal in applyDCEGraphRemovals.
Collaborator
Author
|
This PR also needed to update the metadce side to support the new output format. To properly layer this work, this PR is now based to #27218 and should land after that one. |
guybedford
force-pushed
the
esm-reserved-export-names
branch
from
bb11f8b to
cd89a79
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #27218 (the metadce ESM support), which must land first.
Export names that are JS reserved words (e.g.
default) or otherwise not valid binding identifiers previously produced broken output such asvar default = .../export { default }, which is a syntax error.Such names are now bound to a legal identifier internally (memoized, with a numeric suffix to disambiguate collisions) and exported under their original name via an alias:
Combined with
AUTO_INIT(which frees thedefaultexport slot) this allows a program to expose its owndefaultexport.toValidIdentifier/isValidIdentifier/quoteExportNamehelpers inutility.mjsjsifier.mjsandmodules.mjsbind JS library / runtime symbols to the legalized identifier and emit an aliasedexport { binding as name }when a rename was requiredTested by
test_modularize_instance_reserved_exportintest_other.py, which exports both a reserved-word runtime method (default) and function (delete) and imports them back.Additionally this fixes a metadce interaction with
-sWASM_ESM_INTEGRATIONat-O2+: the aliasedexport { $default as default }was misclassified as a JS function exported to wasm (a wasm import edge).defaultwas recorded as a wasm import, nothing in wasm used it, so it landed inunusedImportsandapplyDCEGraphRemovalsdropped the specifier — leaving a bareexportthat fails to parse in the next pass. A wasm import name is a C symbol and can never bedefault, sodefaultis now excluded from the import-edge classification inemitDCEGraphand from the specifier removal inapplyDCEGraphRemovals. Covered by the extendedemitDCEGraph-esm/applyDCEGraphRemovals-esmfixtures.Resolves #27217.
Made with AI assistance under my review