-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
tools: revert to use @stylistic/eslint-plugin-js v3 #57314
Conversation
@stylistic/eslint-plugin-js v4 has been updated to ship ESM-only, which needs require(esm) support due to the monkey-patching we do in our eslint.config.mjs. The node-test-linter job in Jenkins still runs Node.js v20 which has not yet released unflagging of require(esm). Revert to use v3 for now until we upgrade the Node.js version used in the CI.
Fast-track has been requested by @joyeecheung. Please 👍 to approve. |
Arguably a better fix would be to make this less hacky Lines 33 to 38 in a7f648c
Or to upgrade the CI to use Node.js v22. But the issue is now breaking the CI and this seems like the most no-brainer fix. |
You need to update the |
Ideally PR's such as #57261 would have a Jenkins CI run. |
What do you think about changing the workflow so it also runs with Node.js 20.x so we are more confident about the fix and less likely to have this kind of issues in the future? |
I am not sure how the Jenkins gets the Node.js versions, but if it's an easy fix, happy that someone else get it done ASAP instead. |
On Jenkins the |
Alternative that removes the use of |
node-test-linter CI: https://ci.nodejs.org/job/node-test-linter/59242/ |
I'm good with either approach. thanks for taking a look |
Landing it now to unbreak CI. We can revert it when the Node.js version gets updated or when #57315 lands |
Landed in e05756f |
Refs: #57314 PR-URL: #57315 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@stylistic/eslint-plugin-js v4 has been updated to ship ESM-only, which needs require(esm) support due to the monkey-patching we do in our eslint.config.mjs. The node-test-linter job in Jenkins still runs Node.js v20 which has not yet released unflagging of require(esm). Revert to use v3 for now until we upgrade the Node.js version used in the CI.
Refs: #57261
For an example of broken CI, see https://ci.nodejs.org/job/node-test-linter/59234/console