Navigation Menu

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

Treat "await" as an invalid identifier #4954

Merged
merged 1 commit into from Mar 19, 2017
Merged

Conversation

Jessidhia
Copy link
Member

@Jessidhia Jessidhia commented Dec 7, 2016

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets Fixes #4952 , Fixes #4198
License MIT
Doc PR no
Dependency Changes

"await" is valid (outside async functions) as an identifier in the "script" parse goal, but always invalid in the "module" parse goal. Prevents the transform from generating invalid code when used without a modules-to-script transform.

This does make the function name generated for transform-es2015-function-name not ES2015-compliant, however, but it is probably better than breaking ES2017. An alternative is to make transform-es2015-function-name not give the function a name if it would have been await; that way ES2015 engines could still infer the correct name.

It is valid (outside `async` functions) in the "script" parse goal, but always invalid in the "module" parse goal.

Fixes babel#4952.
@codecov-io
Copy link

codecov-io commented Dec 7, 2016

Current coverage is 89.22% (diff: 100%)

No coverage report found for master at 455d888.

Powered by Codecov. Last update 455d888...b96ec4c

@@ -0,0 +1,3 @@
export {};

var obj = { await: function _await() {} };
Copy link
Member

Choose a reason for hiding this comment

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

but this changes the fn.name

Copy link
Member

@boopathi boopathi Dec 8, 2016

Choose a reason for hiding this comment

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

In chrome this happens - so defining await inside async function is syntax error. Not sure if it's a chrome bug that defining it outside is not a syntax error...

screenshot 2016-12-08 16 32 03

Copy link
Member Author

Choose a reason for hiding this comment

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

The fn.name also changes when it's derived to be arguments or another keyword; not much can be done about it other than omitting the name entirely and leaving it to ES6 engines to correctly derive the name...

And yes, Chrome's error is correct. You cannot use a keyword in a var/let/const binding, and explicitly named functions create a var binding with their name.

Copy link
Member Author

@Jessidhia Jessidhia Dec 8, 2016

Choose a reason for hiding this comment

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

Oh, defining it outside is not a syntax error in strict mode -- it is a syntax error with the module parse goal, which only exists in Chakra(?) and parsers for now.

@hzoo hzoo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Dec 15, 2016
@danez
Copy link
Member

danez commented Dec 22, 2016

This is kindof a duplicate/workaround of #4198

@jquense
Copy link
Contributor

jquense commented Feb 14, 2017

ping! This is breaking our app in Edge. Would love to see it resolved quickly please let me know if there is anything I can contribute to help make that possible.

It seems like @danez s the best solution but seems blocked upstream. Perhaps we could go with a local fix until folks upstream have the time to address it more thoroughly?

Copy link
Member

@danez danez 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 we should go with this.

@Jessidhia Jessidhia merged commit 256fcbc into babel:master Mar 19, 2017
@Jessidhia Jessidhia deleted the patch-3 branch March 19, 2017 02:29
hulkish added a commit to hulkish/babel that referenced this pull request May 2, 2017
* 'master' of github.com:hulkish/babel: (37 commits)
  Increase the code coverage for traverse evaluation (babel#5363)
  Improve options documentation for `babel-plugin-transform-runtime` babel#5401
  [doc] Fix: comments in usage w/ options babel#5400
  document cache option for babel-register babel#5440
  Update coffescript/register reference link address babel#5475
  Update babel-generator's README babel#5517
  Improve example of babel-plugin-transform-es2015-arrow-functions babel#5573
  Remove incorrect docs. babel#5580
  Update transform-es2015-modules-commonjs doc babel#5588
  Fix replacing function declaration in export default (fixes babel#4468) (babel#5456)
  Fix a typo: occurences -> occurrences (babel#5575)
  Fix changelog [skip ci]
  Treat "await" as an invalid identifier (babel#4954)
  Removes unused lodash dependency from babel-helper-builder-react-jsx (babel#5486)
  docs: [skip ci] update documentation
  v6.24.0
  changelog for 6.24.0 [skip ci] (babel#5452)
  Keep parentheses for logical expression when in await expression (fix babel#5428) (babel#5433) (babel#5453)
  Use absolute paths in Babel's CONTRIBUTING.md (babel#5431)
  Fixed broken links in README.md [skip ci] (babel#5449)
  ...
existentialism pushed a commit that referenced this pull request May 19, 2017
It is valid (outside `async` functions) in the "script" parse goal, but always invalid in the "module" parse goal.

Fixes #4952.
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 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 PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
6 participants