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

Switched repo to latest ecmaVersion, except for apps still on requirejs #35651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

orangejenny
Copy link
Contributor

@orangejenny orangejenny commented Jan 21, 2025

Technical Summary

This updates ESLint to use more modern syntax, while retaining the old ecmaVersion for areas that depend on requirejs. The requirejs parser specifically can't handle trailing commas in functions, which are flagged as an error by version 6, so it's useful to have requirejs-dependent code still on that version.

This will result in some new lint errors. I'm not going to proactively address those right now. When I made this change in the web apps webpack PR (#35594), the main change was that ESLint will now flag multi-line function calls that don't use a trailing comma after the last argument (which can be handled automatically by running ESLint with --fix).

This should resolve the kind of lint errors that come up here.

Safety Assurance

Safety story

Only affects linting.

Automated test coverage

No

QA Plan

No

Rollback instructions

  • This PR can be reverted after deploy with no further considerations

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@orangejenny orangejenny added the product/invisible Change has no end-user visible impact label Jan 21, 2025
@orangejenny orangejenny requested a review from millerdev January 21, 2025 22:15
@dimagimon dimagimon added the Risk: Medium Change affects files that have been flagged as medium risk. label Jan 21, 2025
@orangejenny orangejenny requested a review from gherceg January 21, 2025 22:16
@orangejenny orangejenny changed the title Switched repo to latest ecmaVersion, with a few app exceptions Switched repo to latest ecmaVersion, except for apps still on requirejs Jan 21, 2025
Copy link
Contributor

@MartinRiese MartinRiese left a comment

Choose a reason for hiding this comment

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

Hopefully that makes my IDE shut up about this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/invisible Change has no end-user visible impact Risk: Medium Change affects files that have been flagged as medium risk.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants