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

Incorrect ecmaVersion suggests only the old-style ES versions #7405

Closed
mgol opened this issue Oct 19, 2016 · 8 comments · Fixed by eslint/espree#303
Closed

Incorrect ecmaVersion suggests only the old-style ES versions #7405

mgol opened this issue Oct 19, 2016 · 8 comments · Fixed by eslint/espree#303
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion blocked This change can't be completed until another issue is resolved core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@mgol
Copy link
Contributor

mgol commented Oct 19, 2016

Tell us about your environment

  • ESLint Version: 3.8.1
  • Node Version: 3.9.0
  • npm Version: 3.10.8

What parser (default, Babel-ESLint, etc.) are you using?
default

Please show your full configuration:

'use strict';

module.exports = {
    parserOptions: {
        ecmaVersion: 2018,
    },
};

What did you do? Please include the actual source code causing the issue.

eslint .

What did you expect to happen?
An error message that would mention the 2015, 2016 & 2017 options.

What actually happened? Please include the actual, raw output from ESLint.
It prints:

Parsing error: ecmaVersion must be 3, 5, 6, 7, or 8
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Oct 19, 2016
@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Oct 19, 2016
@not-an-aardvark
Copy link
Member

Thanks, I can reproduce this. We should probably mention the 2015-2017 options as well, although I'm not sure of the best way to do that (something like 3, 5, 6, 7, 8, 2015, 2017, or 2017 might be confusing since some of the options will do the same thing).

@gyandeeps
Copy link
Member

We are printing the error message coming from espree.
eslint/espree#300 will help us fix this issue.

@not-an-aardvark
Copy link
Member

At the moment, ESLint modifies the version number here before sending it to espree, so we would have to make a change to that logic as well to avoid confusing errors (e.g. if ecmaVersion: 2018 was passed to ESLint, espree would return the error message for 9).

@mgol
Copy link
Contributor Author

mgol commented Oct 19, 2016

At the moment, ESLint modifies the version number here before sending it to espree

Is there any reason why this algorithm is so generic? I understand mapping 6 to 2015 and 7 to 2016 but why not use the years only going forward?

@nzakas nzakas added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint and removed bug ESLint is working incorrectly accepted There is consensus among the team that this change meets the criteria for inclusion labels Oct 21, 2016
@nzakas
Copy link
Member

nzakas commented Oct 21, 2016

I actually don't think we should be outputting the possible version numbers at all. It should be fine to just say "Invalid ecmaVersion", otherwise we'll be stuck updating that message every year.

And if this is coming from Espree, then it would have to be changed there, not here.

@sstern6
Copy link
Contributor

sstern6 commented Nov 1, 2016

@nzakas would like to take this on if that's ok.

Correct me if I am wrong, but looks like the error message needs to be changed in espree to "Invalid ecmaVersion", here.. https://github.com/eslint/espree/blob/master/espree.js#L126?

Thanks

@nzakas nzakas added the blocked This change can't be completed until another issue is resolved label Nov 2, 2016
@nzakas
Copy link
Member

nzakas commented Nov 2, 2016

@sstern6 yes, you are correct.

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Nov 2, 2016
@sstern6
Copy link
Contributor

sstern6 commented Nov 2, 2016

@nzakas thanks!

Should have a PR open for this late today PST and will ref this issue when I do.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion blocked This change can't be completed until another issue is resolved core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants