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

The keyword 'await' is reserved #564

Closed
Hypercubed opened this issue Mar 21, 2016 · 28 comments · Fixed by #709
Closed

The keyword 'await' is reserved #564

Hypercubed opened this issue Mar 21, 2016 · 28 comments · Fixed by #709

Comments

@Hypercubed
Copy link

Not sure if this is a bug in rollup or if there is some restriction in using reserved words in objects. The strange thing is the error only occurs if the value is a function:

export const test = {
  // await: 123,                 // works!
  // [`await`]: a => 2 * a,      // works!
  await: a => 2 * a,             // Error parsing [path]/utils.js: The keyword 'await' is reserved (8:18) in [path]/utils.js
  add: (n, m) => n + m
}

See here:
https://github.com/Hypercubed/rollup-starter-project/blob/keyword-bug/lib/utils.js#L5

@Hypercubed
Copy link
Author

Appears to only happen when the value is an anonymous function and using the babel plugin.

export const test = {
  // await: 123,                 // works!
  // [`await`]: a => 2 * a,      // works!
  // await: a => 2 * a,          // Error parsing [path]/utils.js: The keyword 'await' is reserved (8:18) in [path]/utils.js
  // await: function (a) {        // again, The keyword 'await' is reserved.
  //  return 2 * a;
  // },
  await: function something (a) { // works!
    return 2 * a;
  },
  add: (n, m) => n + m
}

@Hypercubed
Copy link
Author

Apparently this has been seen before.... d3/d3-queue#55

@eventualbuddha
Copy link
Contributor

I can't seem to reproduce the error. Here's what I did:

$ mkdir rollup-564
$ cd rollup-564
$ npm install rollup
/Users/donovan/Desktop/rollup-564
└─┬ rollup@0.25.4 
  ├─┬ chalk@1.1.1 
  │ ├─┬ ansi-styles@2.2.0 
  │ │ └── color-convert@1.0.0 
  │ ├── escape-string-regexp@1.0.5 
  │ ├─┬ has-ansi@2.0.0 
  │ │ └── ansi-regex@2.0.0 
  │ ├── strip-ansi@3.0.1 
  │ └── supports-color@2.0.0 
  ├── minimist@1.2.0 
  └─┬ source-map-support@0.3.3 
    └─┬ source-map@0.1.32 
      └── amdefine@1.0.0
…snip…
$ cat <<EOS > index.js
export const test = {
  // await: 123,                 // works!
  // [`await`]: a => 2 * a,      // works!
  await: a => 2 * a,             // Error parsing [path]/utils.js: The keyword 'await' is reserved (8:18) in [path]/utils.js
  add: (n, m) => n + m
}
EOS
$ ./node_modules/.bin/rollup index.js
const test = {
  // await: 123,                 // works!
  // [`await`]: a => 2 * a,      // works!
  await: a => 2 * a,             // Error parsing [path]/utils.js: The keyword 'await' is reserved (8:18) in [path]/utils.js
  add: (n, m) => n + m
}

export { test };

Here's my relevant version info:

$ npm version
{ npm: '3.7.3',
  ares: '1.10.1-DEV',
  http_parser: '2.6.2',
  icu: '56.1',
  modules: '47',
  node: '5.9.0',
  openssl: '1.0.2g',
  uv: '1.8.0',
  v8: '4.6.85.31',
  zlib: '1.2.8' }

What happens when you try the same thing?

@Hypercubed
Copy link
Author

Following your setup I see no error. However after adding the babel plugin and the es2015-rollup preset I see the error I described.

./node_modules/.bin/rollup -c rollup.config.js -o out.js
Error parsing /Users/jayson/workspace/temp/rollup-564/index.js: The keyword 'await' is reserved (4:18) in /Users/jayson/workspace/temp/rollup-564/index.js
Type rollup --help for help, or visit https://github.com/rollup/rollup/wiki

Gist:
https://gist.github.com/Hypercubed/e9ca6a4ec4fe6c3bee98

@eventualbuddha
Copy link
Contributor

I'll take a look.

@Hypercubed
Copy link
Author

Babel-cli does not complain:

babel index.js 
export var test = {
  // await: 123,                 // works!
  // [`await`]: a => 2 * a,      // works!
  await: function await(a) {
    return 2 * a;
  }, // Error parsing [path]/utils.js: The keyword 'await' is reserved (8:18) in [path]/utils.js
  add: function add(n, m) {
    return n + m;
  }
};

@eventualbuddha
Copy link
Contributor

It's the await in function await that it's complaining about. Apparently babylon and whatever version of acorn is running on http://cpojer.github.io/esprima_ast_explorer/ is cool with it, but rollup's is not. Will investigate further. Please post here if you discover any more, @Hypercubed.

@eventualbuddha
Copy link
Contributor

Okay, I tracked it down a bit more and found acornjs/acorn#326, which implies that the code generated by babel is invalid. Therefore, I believe this is actually a babel bug.

@eventualbuddha
Copy link
Contributor

So await is invalid as an identifier in a module, but not in a script. Since babel outputs scripts, technically it's (maybe?) not a babel bug. I'm not really sure what rollup should do in this situation.

@Hypercubed
Copy link
Author

"invalid as an identifier" means var await = .... But my understanding (which is very little in this case unfortunately) is that reserved words are allowed as properties.

@eventualbuddha
Copy link
Contributor

As properties, yes. But the part acorn is complaining about is the function name.

@Hypercubed
Copy link
Author

Ok, so properties are "IdentifierNames" while function names are "Identifiers". When Babel encounters an anonymous function it transpiles to using the IdentifierName as the function name. (https://babeljs.io/repl/#?evaluate=false&presets=es2015&experimental=true&loose=false&spec=false&code=%0A%0Aexport%20const%20test%20%3D%20%7B%0A%20%20await%3A%20a%20%3D%3E%202%20*%20a%0A%7D). I think I have see this listed somewhere before as unexpected behavior.

Not really a rollup bug, it's just complaining about bad Babel output.

@Hypercubed
Copy link
Author

Specifically "babel-plugin-transform-es2015-function-name" is doing the naming.

@Hypercubed
Copy link
Author

Adding:

var functionNamePlugin = relative( 'babel-plugin-transform-es2015-function-name', baseLocation );
plugins.splice( plugins.indexOf( functionNamePlugin ), 1 );

to babel-preset-es2015-rollup resolves this issue... unsure if it creates more.

@Hypercubed
Copy link
Author

I submitted a bug to babel. https://phabricator.babeljs.io/T7235 I'm guessing the correct behavior is for babel-plugin-transform-es2015-function-name to not name a function a reserved word. Alternatively babel-preset-es2015-rollup can remove the 'babel-plugin-transform-es2015-function-name' plugin. The output looks good to me with 'babel-plugin-transform-es2015-function-name' plugin removed:

var test = {
  await: function (a) {
    return 2 * a;
  }
};

@Hypercubed
Copy link
Author

I'd be happy to submit a PR to babel-preset-es2015-rollup if we think that is where it belongs... @eventualbuddha @Rich-Harris

@eventualbuddha
Copy link
Contributor

Ideally this should be fixed in babel, though I don't know what their stance will be on whether this is a bug. Another possibility is to replace acorn with babylon inside rollup. And yet another possibility is to set allowReserved: true to simply allow possibly-invalid identifiers, trusting that the user knows what they're doing. I can own following any of these paths if you want to give some guidance, @Rich-Harris.

@Hypercubed
Copy link
Author

Also pretty easy to disable function renaming in the babel rollup preset: https://github.com/rollup/babel-preset-es2015-rollup/blob/master/index.js

I don't really see the benefit of the renaming plugin... Maybe it is necessary for es2016 compliance?

I'm pretty sure very few people are encountering this issue.

@eventualbuddha
Copy link
Contributor

@Hypercubed sure, but that's behavior that people may have been relying on. I'm not sure how exactly, and maybe there's no behavioral difference and it's just a nice thing for stack traces. Enabling the allowReserved: true flag is probably the easiest thing and should not cause anyone's working builds to start failing.

@Hypercubed
Copy link
Author

True, and it seems that the naming is part of the spec... But I'd imagine if anyone is relying on that they would have trouble after minification.

@jehiah
Copy link

jehiah commented Mar 27, 2016

@Hypercubed thanks for digging in further on this than i got in d3/d3-queue#55

@Rich-Harris
Copy link
Contributor

👍 to allowReserved: true – function names are very useful for stack traces, would be a shame to lose them. Perhaps this needs a warning to go with it?

@eventualbuddha
Copy link
Contributor

You mean a doc warning?

On Mar 26, 2016, at 8:46 PM, Rich Harris notifications@github.com wrote:

to allowReserved: true – function names are very useful for stack traces, would be a shame to lose them. Perhaps this needs a warning to go with it?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

@Rich-Harris
Copy link
Contributor

I was thinking something like this, logged to the console during bundling:

123: var methods = {
124:   await: function await () {
                       ^----
Reserved word used as identifier in foo.js line 124. This is a syntax error in ES modules
and may cause problems in future; consider renaming the function to prevent bugs

Of course ideally this category of warning would be handled in a consistent fashion, i.e we have some helpers for logging syntax errors with code snippets (can't remember where we previously discussed this) – this case is a bit tricky because whereas you generally want to refer back to the original source (i.e. trace sourcemaps), here the problems is introduced during transformation...

@Hypercubed
Copy link
Author

Hypercubed commented Apr 19, 2016

BTW @Rich-Harris :

My solution for babel that avoids this issue with rollup/acorn (using babel) is:

var test = {
  [`await`]: function (a) {
    return 2 * a;
  }
};

But, this causes an error in buble: "Computed properties are not supported"

😞

@Rich-Harris
Copy link
Contributor

I don't know if Babel still has this bug, but I've raised #709 as a possible solution

@Rich-Harris
Copy link
Contributor

Actually, looking into this further, it may be a bug with Acorn. Doesn't look like there's a problem with function await () {} in browsers (surprisingly?).

@Rich-Harris
Copy link
Contributor

Just released 0.31.0 which lets you specify options to pass to Acorn:

rollup.rollup({
  entry: 'src/main.js',
  acorn: {
    allowReserved: true
  }
}).then( bundle => ... );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants