Support ESM export names that are not valid JS identifiers#27236
Support ESM export names that are not valid JS identifiers#27236guybedford wants to merge 2 commits into
Conversation
sbc100
left a comment
sbc100
left a comment
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?
It complements it - that is only called for
It is now supported and tested in this PR.
As above, |
|
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. |
bb11f8b to
cd89a79
Compare
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.
cd89a79 to
fd0ac6e
Compare
2 participants
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