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

Properly parse export default from when exportDefaultFrom is not enabled #11676

Merged
merged 6 commits into from Jun 5, 2020

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jun 4, 2020

Q                       A
Fixed Issues? Fixes #7293, closes #7309
Patch: Bug Fix? Yes
Tests Added + Pass? Yes
License MIT

This PR contains two fixes. I include these two because they are highly related and the change should be acceptable to fit in a single PR.

The first fix addresses to export default from; when exportDefaultFrom is not enabled, which is allowed by the spec. We do a second lookahead after we have seen export default from via a new nextTokenStartSince method, and test if it starts with ' or " which iff. starts a string literal.

The second fix ban export default from when exportDefaultFrom is enabled, which is forbidden by the spec draft. Though I don't find any detailed explainers but I guess it is to address the following ASI hazard.

export default from
"bar";

@JLHwung JLHwung added PR: Spec Compliance 👓 A type of pull request used for our changelog categories PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: export-default-from labels Jun 4, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 4, 2020

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 a6138fb:

Sandbox Source
snowy-bash-xqz22 Configuration
still-browser-iyjen Configuration

@babel-bot
Copy link
Collaborator

babel-bot commented Jun 4, 2020

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

@kaicataldo
Copy link
Member

LGTM, but looks like Flow is currently failing the build.

@@ -0,0 +1 @@
export default \u{66}rom;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️IMO it should be addressed in the export-default-from proposal that while export default from is banned, export default \u{66}rom is still valid.

@nicolo-ribaudo
Copy link
Member

Related - tc39/proposal-export-default-from#2

);
return (
nextAfterFrom === charCodes.quotationMark ||
nextAfterFrom === charCodes.apostrophe
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why we need to special-case these two characters and why not, for example ( or *.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we see a string literal after from, it is considered a fromClause and thus it is indeed export default fromClause instead of export default AssignmentExpression.

Copy link
Member

Choose a reason for hiding this comment

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

I was reading quaotationMark as questionMark 😁

packages/babel-parser/src/plugins/placeholders.js Outdated Show resolved Hide resolved
Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
@existentialism existentialism merged commit 71d3527 into babel:master Jun 5, 2020
@existentialism existentialism deleted the fix-7293 branch June 5, 2020 14:08
@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 Sep 5, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2020
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 PR: Bug Fix 🐛 A type of pull request used for our changelog categories PR: Spec Compliance 👓 A type of pull request used for our changelog categories Spec: export-default-from
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'export default from' can not be correctly compiled
5 participants