Skip to content

String import/export specifier #12091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
nicolo-ribaudo merged 25 commits into from
Oct 14, 2020
Merged

String import/export specifier #12091

nicolo-ribaudo merged 25 commits into from
Oct 14, 2020

Conversation

@JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Sep 22, 2020 •
edited by gitpod-io bot
Loading

Q                       A
Fixed Issues? Implements tc39/ecma262#2154
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link babel/website#2396
Any Dependency Changes? @babel/helper-module-transforms now depends on @babel/helper-validator-identifier
License MIT

This PR implements support of Arbitrary module namespace identifier names behind moduleStringNames parser plugin

import { "some imports" as foo } from "module"
export { foo as "some exports" } from "module"
export { "another imports" as "another exports" } from "module"
export { "rest exports" } from "module"

Babel can now parse such code and transform it to CommonJS/AMD/UMD/SystemJS code. Take CommonJS as an output example

const m = require("module");
foo = m["some imports"];
exports["some exports"] = m.foo;
exports["another exports"] = m["another imports"];
exports["rest exports"] = m["rest exports"];

A new package @babel/plugin-syntax-module-string-names is introduced. See also https://github.com/babel/babel/pull/12091/files#diff-ef433cb06c064c1e7a5af0f448d98a78 for AST spec changes.

Todo:

  • umd-transform
  • amd-transform
  • systemjs-transform (/cc @guybedford)

@JLHwung JLHwung added PR: Spec Compliance 👓 A type of pull request used for our changelog categories PR: New Feature 🚀 A type of pull request used for our changelog categories labels Sep 22, 2020
@babel-bot
Copy link
Collaborator

babel-bot commented Sep 22, 2020 •
edited
Loading

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/29849/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 22, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 107444b:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@JLHwung JLHwung added this to the v7.12.0 milestone Sep 22, 2020
@JLHwung JLHwung force-pushed the string-module-specifier-name branch from bd69ecd to 425637a Compare September 22, 2020 20:56
@JLHwung JLHwung marked this pull request as ready for review September 22, 2020 21:01
@JLHwung JLHwung mentioned this pull request Sep 25, 2020
Merged
@JLHwung JLHwung force-pushed the string-module-specifier-name branch 2 times, most recently from d7f2778 to c50c767 Compare September 25, 2020 21:07
guybedford
guybedford approved these changes Sep 26, 2020
View reviewed changes
Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

SystemJS output looks great, and glad it fixed some bugs too.

nicolo-ribaudo
nicolo-ribaudo reviewed Sep 26, 2020
View reviewed changes
nicolo-ribaudo
nicolo-ribaudo reviewed Sep 26, 2020
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I think that we should introduce this behind a syntax plugin. Even if the spec PR has consensus, it will still take a while to be merged because it doesn't have tests.

Also, for example tc39/ecma262#1174 still took a few months after having consensus and tests.

@JLHwung JLHwung force-pushed the string-module-specifier-name branch from c50c767 to c63fd3c Compare September 28, 2020 15:39
@JLHwung JLHwung changed the title String module specifier name String import/export specifier Sep 28, 2020
@JLHwung JLHwung force-pushed the string-module-specifier-name branch 4 times, most recently from 7ba178b to 6f141a2 Compare September 30, 2020 12:59
@JLHwung JLHwung added the PR: Needs Review A pull request awaiting more approvals label Oct 8, 2020
nicolo-ribaudo
nicolo-ribaudo approved these changes Oct 9, 2020
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Except for a question (maybe small error) this looks good 👍

packages/babel-parser/ast/spec.md
exported: Identifier;
local?: Identifier;
exported: Identifier | StringLiteral;
local?: Identifier | StringLiteral;
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo Oct 9, 2020

Choose a reason for hiding this comment

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

When can local be a string?

Copy link
Contributor Author

@JLHwung JLHwung Oct 9, 2020

Choose a reason for hiding this comment

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

export { "😄" as "😂" } from "emojis"

@JLHwung JLHwung force-pushed the string-module-specifier-name branch from f71402b to 6e0d7bf Compare October 9, 2020 18:37
kaicataldo
kaicataldo approved these changes Oct 10, 2020
View reviewed changes
@nicolo-ribaudo nicolo-ribaudo removed the PR: Needs Review A pull request awaiting more approvals label Oct 11, 2020
@JLHwung JLHwung force-pushed the string-module-specifier-name branch from 6e0d7bf to 0b4dc58 Compare October 12, 2020 17:59
existentialism
existentialism reviewed Oct 12, 2020
View reviewed changes
existentialism
existentialism reviewed Oct 12, 2020
View reviewed changes
existentialism
existentialism reviewed Oct 12, 2020
View reviewed changes
existentialism
existentialism approved these changes Oct 12, 2020
View reviewed changes
Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

Looks great! Just some 🚲shedding on error messages.

@existentialism existentialism added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Oct 12, 2020
@nicolo-ribaudo nicolo-ribaudo merged commit cb30c1d into babel:main Oct 14, 2020
@nicolo-ribaudo nicolo-ribaudo deleted the string-module-specifier-name branch October 14, 2020 17:59
nicolo-ribaudo pushed a commit that referenced this pull request Oct 14, 2020
@JLHwung @existentialism @nicolo-ribaudo
String import/export specifier (#12091)
21d7ee2 
* feat: parse moduleExportName
* feat: add validators
* Support string specifier name in commonjs transform
* Support string specifier name in export-ns-from
* test: add loose testcases
* test: add testcases for amd and umd
* feat: support systemjs
* test: update fixtures fixed in #12110
* add plugin name typings
* test: rename test layout
* feat: implement under moduleStringNames flag
* chore: add plugin syntax module string names
* feat: support ModuleExportName as ModuleExportName
* test: update test fixtures
* fix flow errors
* docs: update AST spec
* feat: support { "some imports" as "some exports" }
* feat: support { "some imports" as "some exports" } in systemjs
* test: add test on `import { "foo" }`
* Address review comments
* add moduleStringNames to missing plugin helpers
* Apply suggestions from code review
* update test fixtures
* Update packages/babel-parser/src/parser/error-message.js
* update test fixtures

Co-Authored-By: Kai Cataldo <[email protected]>
Co-authored-by: Brian Ng <[email protected]>
@fisker fisker mentioned this pull request Oct 15, 2020
Merged
3 tasks
@ExE-Boss ExE-Boss mentioned this pull request Oct 22, 2020
Closed
5 tasks
@alangpierce alangpierce mentioned this pull request Dec 3, 2020
Open
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@existentialism existentialism existentialism approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

+1 more reviewer

@guybedford guybedford guybedford approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release PR: Spec Compliance 👓 A type of pull request used for our changelog categories

Projects

None yet

Milestone

v7.12.0

Development

Successfully merging this pull request may close these issues.

6 participants

@JLHwung @babel-bot @existentialism @guybedford @nicolo-ribaudo @kaicataldo