fix(systemjs): support __proto__ as an export name#18032
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61683 |
There was a problem hiding this comment.
Pull request overview
Updates @babel/plugin-transform-modules-systemjs to correctly handle exports named __proto__ by ensuring generated export namespace objects treat __proto__ as a normal data property (avoiding prototype mutation semantics) and by adding/adjusting fixtures to cover these cases.
Changes:
- Emit computed export keys for
__proto__in_export({ ... })object payloads (e.g.{ ["__proto__"]: value }). - Initialize export-star aggregation objects with
{ __proto__: null }to ensure later assignments to.__proto__become ordinary properties. - Add new SystemJS fixture coverage for
export ... fromscenarios involving__proto__(including a case withtransform-computed-properties).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/babel-plugin-transform-modules-systemjs/src/index.ts | Adjusts export object construction to safely represent __proto__ (computed keys + null-proto export-star objects). |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-named-alongside-with-export-star/output.mjs | Updates expected output to use { __proto__: null } export-star aggregation object. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from/output.mjs | Updates expected output to use { __proto__: null } export-star aggregation object. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-string/input.mjs | Adds fixture input covering re-exporting to "__proto__" via string export name. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-string/output.mjs | Adds expected output using computed ["__proto__"] key in _export({ ... }). |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-name/input.mjs | Adds fixture input covering export { __proto__ } from "foo". |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-name/output.mjs | Adds expected output using computed ["__proto__"] key in _export({ ... }). |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-name-with-computed-properties-transform/options.json | Adds fixture options to run SystemJS transform plus computed-properties transform. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-name-with-computed-properties-transform/input.mjs | Adds fixture input for __proto__ re-export under computed-properties transform. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-proto-name-with-computed-properties-transform/output.mjs | Adds expected output showing __proto__ handled via defineProperty helper. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-string/input.mjs | Adds fixture input covering default + "__proto__" re-exports. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-string/output.mjs | Adds expected output using computed ["__proto__"] key. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-string-and-all/input.mjs | Adds fixture input covering export-star plus default + "__proto__" re-exports. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-string-and-all/output.mjs | Adds expected output using { __proto__: null } export-star aggregation object + safe assignment. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-name/input.mjs | Adds fixture input covering default + __proto__ (identifier) re-exports. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-name/output.mjs | Adds expected output using computed ["__proto__"] key. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-name-and-all/input.mjs | Adds fixture input covering export-star plus default + __proto__ re-exports. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/systemjs/export-from-default-and-proto-name-and-all/output.mjs | Adds expected output using { __proto__: null } export-star aggregation object + safe assignment. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/interop-module-string-names/export-named-string-can-be-identifier-with-export-star/output.mjs | Updates expected output to use { __proto__: null } export-star aggregation object. |
| packages/babel-plugin-transform-modules-systemjs/test/fixtures/interop-module-string-names/export-named-from-with-export-star/output.mjs | Updates expected output to use { __proto__: null } export-star aggregation object. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
commit: |
| t.variableDeclarator( | ||
| t.identifier(exportObj), | ||
| t.objectExpression([ | ||
| t.objectProperty(t.identifier(protoKey), t.nullLiteral()), |
There was a problem hiding this comment.
Do we need this even if there is no __proto__ export?
There was a problem hiding this comment.
| var _exportObj = { | ||
| __proto__: null | ||
| }; | ||
| for (var _key in _otherMjs) { |
There was a problem hiding this comment.
Initialize exportObj with { proto: null } when transforming export *
When transforming export *, we don't know at compile time if the imported module _otherMjs contains __proto__. We could do that in the runtime here but we will have to specialize __proto__ and call Object.defineProperty, which further bloats the output.
Note that the _exportObj will only be created when there is a star export.
__proto__.In this PR we implemented two output changes:
exportObjwith{ __proto__: null }when transformingexport *export { __proto__ }to_exports({ ["__proto__"]: ... })These changes ensure that the
__proto__will be assigned as a normal object property of the namespace object.