Skip to content
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.

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

Split exportExtensions into exportDefault and exportNamespace plugins… #6920

Merged
merged 3 commits into from Nov 30, 2017

Conversation

existentialism
Copy link
Member

@existentialism existentialism commented Nov 27, 2017

… in babylon

Q                       A
Fixed Issues? Fixes #6678
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? y/y
Documentation PR
Any Dependency Changes?
License MIT

Trying to clean up some v7 TODOs.

Was chatting with @hzoo but we could rename the plugins (and resulting transforms) to export-default-from and export-namespace-from to get a slightly more clear error:

This experimental syntax requires enabling the parser plugin: 'exportDefaultFrom'

But probably not a huge deal?

Also, I removed some redundant tests in Babylon.

@existentialism existentialism added pkg: parser PR: Spec Compliance 👓 A type of pull request used for our changelog categories labels Nov 27, 2017
@@ -0,0 +1,35 @@
# @babel/plugin-syntax-export-default

> Allow parsing of export default from.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: export default from should be in backticks so that it is formatted as code? (export default from)

@@ -0,0 +1,35 @@
# @babel/plugin-syntax-export-namespace

> Allow parsing of export namespace from.
Copy link
Member

Choose a reason for hiding this comment

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

Also here

@babel-bot
Copy link
Collaborator

babel-bot commented Nov 27, 2017

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

@@ -158,6 +159,8 @@ registerPlugins({
"proposal-class-properties": require("@babel/plugin-proposal-class-properties"),
"proposal-decorators": require("@babel/plugin-proposal-decorators"),
"proposal-do-expressions": require("@babel/plugin-proposal-do-expressions"),
"proposal-export-default": require("@babel/plugin-proposal-export-default"),
Copy link
Member Author

Choose a reason for hiding this comment

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

@Daniel15 I noticed standalone was including syntax-export-extensions, but not the actual transforms. Any reason not to?

Copy link
Member

Choose a reason for hiding this comment

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

well one thing is that the repl uses stage-x but none of the plugins themselves since not sure who is using it that way

Copy link
Member

Choose a reason for hiding this comment

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

@existentialism Yeah this is fine. It might have just been an oversight. I have an item on my todo list to add a script that verifies that every syntax + transform plugin in the monorepo is listed here too. Maybe I'll file a task for that and let someone else do the work 😛

@nicolo-ribaudo
Copy link
Member

+1 for exportDefaultFrom instead of exportDefault

@hzoo
Copy link
Member

hzoo commented Nov 29, 2017

i'm cool with the longer name

@@ -1 +1 @@
{ "plugins": ["exportExtensions"] }
Copy link
Member

Choose a reason for hiding this comment

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

later: we could probably rename these test files

@existentialism
Copy link
Member Author

@hzoo @nicolo-ribaudo could use a quick re-review, added -from to transforms and plugins

@@ -0,0 +1,35 @@
# @babel/plugin-syntax-export-namespace-from

> Allow parsing of `export namespace from`.
Copy link
Member

Choose a reason for hiding this comment

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

export * as namespace from

@@ -107,7 +107,7 @@ const options = {
const flowOptionsMapping = {
esproposal_class_instance_fields: "classProperties",
esproposal_class_static_fields: "classProperties",
esproposal_export_star_as: "exportExtensions",
esproposal_export_star_as: "exportNamespace",
Copy link
Member

Choose a reason for hiding this comment

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

exportNamespaceFrom

@loganfsmyth
Copy link
Member

I think we should consider adding an alias to

function getParserClass(
to check if exportExtensions is given, and auto-expand it to the two separate flags, to help things migrate.

@hzoo hzoo merged commit d8bbaaa into master Nov 30, 2017
@hzoo
Copy link
Member

hzoo commented Nov 30, 2017

Yeah that's a good idea, should be fine in another PR

@existentialism existentialism deleted the issue6678 branch November 30, 2017 22:47
@hzoo hzoo mentioned this pull request Dec 1, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Spec Compliance 👓 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split export extensions into 2
6 participants