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

Allow ecmaVersion: latest #495

Closed
nzakas opened this issue May 6, 2021 · 19 comments
Closed

Allow ecmaVersion: latest #495

nzakas opened this issue May 6, 2021 · 19 comments

Comments

@nzakas
Copy link
Member

nzakas commented May 6, 2021

As of v8.0.0, Acorn now allows ecmaVersion: latest, which automatically sets ecmaVersion to the latest available version implemented.

Can we also allow this for Espree?

@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage May 6, 2021
@nzakas nzakas moved this from Needs Triage to Evaluating in Triage May 6, 2021
@aladdin-add
Copy link
Member

to clarify: ecmaVersion: "latest" is as the same as ecmaVersion: require("espree").latestEcmaVersion. not passing the "latest" to acorn options?

@nzakas
Copy link
Member Author

nzakas commented May 8, 2021

No, Acorn supports “latest” so we can pass that directly through from Espree.

@mysticatea
Copy link
Member

How about making the latest as default?

I'm not sure if ES5 is the most popular environment now. Plus, eslint-plugin-es or something like provides more readable errors instead of "unexpected token" if people want to restrict to an old environment.

@aladdin-add
Copy link
Member

How about making the latest as default?

I'm not sure if ES5 is the most popular environment now. Plus, eslint-plugin-es or something like provides more readable errors instead of "unexpected token" if people want to restrict to an old environment.

As of now, I don't think we can go that far. maybe we can reconsider it after the "latest" option get landed.

@aladdin-add
Copy link
Member

aladdin-add commented May 8, 2021

No, Acorn supports “latest” so we can pass that directly through from Espree.

it could cause potential confusing if the two values are not equal.

@nzakas
Copy link
Member Author

nzakas commented May 11, 2021

@aladdin-add what problems are you thinking of? The value inside Acorn doesn’t have any effect on ESLint.

I think changing the default to latest makes sense. This should not affect ESLint because we are manually setting ecmaVersion in ESLint itself.

@aladdin-add
Copy link
Member

aladdin-add commented May 11, 2021

an example: the latest espree's latestEcmaVersion is 2021, but passing {"ecmaVersion": "latest"} will also allow es2022 features.

@nzakas
Copy link
Member Author

nzakas commented May 11, 2021

@aladdin-add Acorn doesn't implement experimental features, so I'm not sure if that's true. Can you give an example where Acorn is supporting something after 2021?

@aladdin-add
Copy link
Member

class fields - it has been supported in acorn v8.2.0, but not for espree yet (#486 has not been merged & released).

@nzakas
Copy link
Member Author

nzakas commented May 12, 2021

Ah right. Then we should update latestEcmaVersion too before the next Espree release.

@mysticatea
Copy link
Member

Yes. #486 updates latestEcmaVersion (tests/lib/supported-ecmaversions.js). It comes from the last value of supportedEcmaVersions. Therefore, we cannot support new ecma versions unless latestEcmaVersion update.

@nzakas
Copy link
Member Author

nzakas commented May 20, 2021

Initial implementation here: #499

Note: this issue isn't yet accepted, so I just sent the PR as a draft for now.

@mdjermanovic
Copy link
Member

No, Acorn supports “latest” so we can pass that directly through from Espree.

Just to double-check, as it isn't clear from the discussion - we decided to pass Espree's latestEcmaVersion (number) instead of "latest" to Acorn?

@mdjermanovic
Copy link
Member

I think changing the default to latest makes sense. This should not affect ESLint because we are manually setting ecmaVersion in ESLint itself.

If user doesn't specify parserOptions.ecmaVersion in their ESLint config, ESLint will pass ecmaVersion: undefined to Espree, so in that case Espree's default applies. This change would be then a breaking change for ESLint users (default for ecmaVersion changes from 5 to the latest).

@mdjermanovic
Copy link
Member

Do we intend to allow specifying ecmaVersion: "latest" in ESLint config? I think that's the idea in rfcs#79 - 3. Remove the usage of eslint & espree.

A problem with ecmaVersion: "latest" is that eslint-scope also expects ecmaVersion. ESLint calls eslintScope.analyze with parserOptions.ecmaVersion from the configuration (or 5 if it wasn't specified).

https://github.com/eslint/eslint/blob/master/lib/linter/linter.js#L607-L620

If we pass "latest" from ESLint, scope will be analyzed as if it was ecmaVersion: 3.

@aladdin-add
Copy link
Member

aladdin-add commented May 24, 2021

to avoid break eslint, sounds we should explicitly normalize the ecmaVersion to 5 in eslint (if not specified).

as for "latest", we have 2 choices:

  • convert "latest" to espree.latestEcmaVersion, and passing it to eslintScope.analyze
  • pass "latest" to eslintScope.analyze (it need updates to allow the value)

@nzakas
Copy link
Member Author

nzakas commented May 24, 2021

Yes, the idea is to allow "latest" in ESLint configs eventually, but that doesn't have to be done along with this change in Espree. We can always address that later and just make sure that ESLint defaults to ecmaVersion: 5 when no other setting is present. We don't have to allow "latest" right away in ESLint (I'm pretty sure it will be a validation error right now).

The value for ecmaVersion in eslint-scope is only to enable let/const block-scope evaluations. It just checks if ecmaVersion > 5. As @aladdin-add pointed out, we can always normalize "latest" in ESLint to espree.latestEcmaVersion to avoid that.

I don't think any of this should block enabling "latest" for Espree, though.

@aladdin-add
Copy link
Member

aladdin-add commented May 25, 2021

to allow "latest" in ESLint configs, this is not a must. I have made a PR to implement it: eslint/eslint#14622

@mdjermanovic
Copy link
Member

I support both "latest" and changing the default to latest (assuming that we will update ESLint to keep ecmaVersion: 5 as ESLint's default) 👍

@nzakas nzakas moved this from Evaluating to Pull Request Opened in Triage May 27, 2021
@nzakas nzakas closed this as completed in b068cea Jun 9, 2021
Triage automation moved this from Pull Request Opened to Complete Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Triage
Complete
Development

No branches or pull requests

4 participants