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

Bump Acorn JS version to ES2018 #11435

Merged
merged 3 commits into from
Jun 17, 2020
Merged

Bump Acorn JS version to ES2018 #11435

merged 3 commits into from
Jun 17, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Jun 17, 2020

The acorn code says

  // `ecmaVersion` indicates the ECMAScript version to parse. Must be
  // either 3, 5, 6 (2015), 7 (2016), 8 (2017), 9 (2018), or 10
  // (2019). This influences support for strict mode, the set of
  // reserved words, and support for new syntax features. The default
  // is 9.

We set the version to 6 (2015) when we added Acorn, probably to
keep it as close as possible to the old pre-Acorn parser. But the
default in our version is 9 (2018) so we may as well switch to that,
and that fixes #11431 (8 - 2017 - would have been enough for
that specific bug).

@RReverser
Copy link
Collaborator

Sounds good; we at Acorn are thinking of bumping the ecmaVersion default in the next major release anyway, but it's best to set it explicitly anyway.

tools/acorn-optimizer.js Outdated Show resolved Hide resolved
Co-authored-by: Ingvar Stepanyan <[email protected]>
@kripken
Copy link
Member Author

kripken commented Jun 17, 2020

Thanks! I didn't know 2018 was an accepted syntax.

I think 2018 is better than 2019 for now, as I'm not sure if 2019 was stable when we branched our version of Acorn. We should probably update.

@RReverser
Copy link
Collaborator

Thanks! I didn't know 2018 was an accepted syntax.

It says so in the comment you quoted, but maybe we didn't make it clear enough that versions in parentheses are aliases :)

@RReverser
Copy link
Collaborator

I think 2018 is better than 2019 for now, as I'm not sure if 2019 was stable when we branched our version of Acorn.

FWIW core Acorn doesn't implement any features that haven't been finalised yet (< stage 4), so you should be safe either way; upgrading sounds like a good idea though, especially since the latest version has stable 2020 support.

@kripken
Copy link
Member Author

kripken commented Jun 17, 2020

Heh, I should read the full text of things I quote I guess 😄

@kripken kripken merged commit 56dc964 into master Jun 17, 2020
@kripken kripken deleted the es2018 branch June 17, 2020 22:33
@sbc100
Copy link
Collaborator

sbc100 commented Jun 17, 2020

On the subject of acorn, what is are blockers on us upstreaming out local patches and switching to the npm version? Would be nice to avoid the local copy.

@kripken
Copy link
Member Author

kripken commented Jun 17, 2020

I believe they just didn't want one of our patches, acornjs/acorn#793 It might be possible to do something non-invasive that achieves the same result, but I don't know.

@RReverser
Copy link
Collaborator

RReverser commented Jun 17, 2020

It might be possible to do something non-invasive that achieves the same result, but I don't know.

Yeah the error object thrown by Acorn contains location in the source it was thrown at, so you don't really need a patch, only a wrapper to try-catch it and format it in whichever way you prefer.

It should be easy enough, but I'm happy to help if you run into any issues.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 18, 2020

I've love to see this happen... I'll open bug and hopefully one of us can get around to it: #11439

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

Successfully merging this pull request may close these issues.

acorn-optimizer.js trips over async functions
4 participants