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

docs: fix ecmaVersion in one example, add checks #18241

Merged
merged 2 commits into from Mar 30, 2024

Conversation

mdjermanovic
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

While testing eslint/eslint.org#509, I noticed that one example for the strict rule has "ecmaVersion": 6 in the configuration. This works mostly well because 6 is an alias for 2015, but when the example is opened in the Playground, nothing is selected in the ECMA Version list:

image

What changes did you make? (Give an overview)

  1. Updated the mentioned configuration to "ecmaVersion": 2015.
  2. Added a new check in tools/check-rule-examples.js - ecmaVersion, if present, must be one of 3, 5, 2015, 2016, ..., 2024 (the latest number). "latest" string is also disallowed because it is unnecessary since it's the default.

Is there anything you'd like reviewers to focus on?

@mdjermanovic mdjermanovic requested a review from a team as a code owner March 28, 2024 13:07
@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Mar 28, 2024
@github-actions github-actions bot removed the documentation Relates to ESLint's documentation label Mar 28, 2024
Copy link

netlify bot commented Mar 28, 2024

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit ed24a74
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/66056d8c4e7601000826ebf9

@eslint-github-bot eslint-github-bot bot added the documentation Relates to ESLint's documentation label Mar 28, 2024
@github-actions github-actions bot removed the documentation Relates to ESLint's documentation label Mar 28, 2024
@aladdin-add
Copy link
Member

but when the example is opened in the Playground, nothing is selected in the ECMA Version list:

Not sure we should update the examples - It seems more of an issue in the playground. Can we fix it in that, I mean, something like normalize the ecmaVersion to year-style, e.g. 6=>2015?

@mdjermanovic
Copy link
Member Author

but when the example is opened in the Playground, nothing is selected in the ECMA Version list:

Not sure we should update the examples - It seems more of an issue in the playground. Can we fix it in that, I mean, something like normalize the ecmaVersion to year-style, e.g. 6=>2015?

Serialized configs are generated by our sites, so I think we don't need to support all variants. That said, the old demo (https://archive.eslint.org/demo) was using the 6, 7, 8... style, thus it's a good idea to normalize ecmaVersion in the Playground to support old links 👍 . I'll add normalization in eslint/eslint.org#509.

But I'd still like to add this check to keep docs' configs consistent. What do you think?

@aladdin-add
Copy link
Member

for consistency, I'm fine with it - seems it's most used in the docs.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Just leaving open for a couple of days for others to get a chance to review.

@fasttime fasttime added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion labels Mar 29, 2024
@fasttime
Copy link
Member

There are 35 rule examples that use "sourceType": "module" redundantly. If "ecmaVersion": "latest" won't be allowed for consistency reasons, are there any plans to also disallow "sourceType": "module" or "jsx": false?

@mdjermanovic
Copy link
Member Author

There are 35 rule examples that use "sourceType": "module" redundantly. If "ecmaVersion": "latest" won't be allowed for consistency reasons, are there any plans to also disallow "sourceType": "module" or "jsx": false?

Yes, I'm planning to do that in another PR that will update links to send config in the flat format now that eslint/eslint.org#509 is merged.

Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 26384d3 into main Mar 30, 2024
19 checks passed
@mdjermanovic mdjermanovic deleted the docs-check-ecmaversion branch March 30, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

4 participants