Skip to content

Upgrade ESLint to version 9#5567

Merged
canova merged 7 commits intofirefox-devtools:mainfrom
canova:eslint-upgrade
Aug 26, 2025
Merged

Upgrade ESLint to version 9#5567
canova merged 7 commits intofirefox-devtools:mainfrom
canova:eslint-upgrade

Conversation

@canova
Copy link
Member

@canova canova commented Aug 25, 2025

This PR upgrades the ESLint version from 8 to 9 by first switching to flat config format because version 9 made this format required.

There is a migration guide here: https://eslint.org/docs/latest/use/configure/migration-guide

I also fixed some minor issues we have inside the eslint config while there.

@canova canova requested a review from mstange August 25, 2025 18:09
@codecov
Copy link

codecov bot commented Aug 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.85%. Comparing base (142890d) to head (cb32d51).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5567   +/-   ##
=======================================
  Coverage   85.84%   85.85%           
=======================================
  Files         309      308    -1     
  Lines       30338    30337    -1     
  Branches     8346     8346           
=======================================
  Hits        26045    26045           
+ Misses       3873     3872    -1     
  Partials      420      420           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

...jestDomPlugin.configs.recommended.rules,

'react/jsx-no-bind': 'off',
// This rule isn't useful because use Flow.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Flow" :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it 😄


constructor(file: string) {
const worker = new Worker(__dirname + '/node-worker-contents.js', {
const worker = new Worker(__dirname + '/node-worker-contents.mjs', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, interesting. #5556 touches this code too but this one should land first.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I wasn't aware that it was touching it. Just saw it. I had to convert this to .mjs so it understands that it's an ES module file. I'm not so sure if that works when we add it as an eval string. But probaby it's fine to keep it as require if it's only kept as a string as eslint won't complain :)

@mstange
Copy link
Contributor

mstange commented Aug 25, 2025

Thank you!

@canova canova force-pushed the eslint-upgrade branch 2 times, most recently from 35fcbc8 to 5e41aa7 Compare August 26, 2025 13:35
@canova
Copy link
Member Author

canova commented Aug 26, 2025

@mstange Actually I was testing it locally and realized that some rules weren't applying properly anymore (it was not complaining about anything even if you broke that rule). So I made sure that they apply properly now. It should be a 1:1 match to our old eslint config. I added 2 fixup commits that I will later rebase and an extra commit. So last 3 commits are new. I did it this way so it's easier to review. Can you check it again?

@canova canova requested a review from mstange August 26, 2025 13:55
canova added 7 commits August 26, 2025 20:09
This is going to help us upgrade the eslint version to 9.
We've been using react 18 for a while, but this setting wasn't updated.
It looks like this has been removed a long while ago and it doesn't do
anything anymore:
eslint/eslint#9990
Currently we are using node 22 which supports 2024.
It looks like this line was a false positive for @babel/no-invalid-this
initially because here `this` comes from a parameter and it's not a
global `this`. After we upgraded the eslint version, this is correctly
caught and not reported as error.

I double checked this rule, by specifically making this mistake in the
codebase, and it looks like we catch it properly elsewhere, so it's not
a case where we fail to detect this rule.

This is the explanation of that rule:
https://eslint.org/docs/latest/rules/no-invalid-this

I explicitly tried the first incorrect example here and `yarn lint-js`
catches it properly.
@canova canova merged commit 65fbff3 into firefox-devtools:main Aug 26, 2025
15 checks passed
@canova canova deleted the eslint-upgrade branch August 26, 2025 19:03
@canova canova mentioned this pull request Sep 2, 2025
canova added a commit that referenced this pull request Sep 2, 2025
Changes:

[Nazım Can Altınova] Display the marker description at the top inside
the marker tooltips (#5534)
[Florian Quèze] Change the 'JavaScript' radio button label to 'Script'
(#5530)
[Markus Stange] Implement profile logic and some selectors for the
function list (#5525)
[Markus Stange] Some small type fixes (#5538)
[Markus Stange] Simplify return type of the callback we pass to
setState. (#5540)
[Markus Stange] Pass the correct value to the reducer's action argument
(#5543)
[Markus Stange] Change withSize to accept PropsWithoutSize as its type
parameter (#5541)
[Nazım Can Altınova] Make sure that the test-debug command runs the
tests properly (#5545)
[Markus Stange] Improve type coverage involving network phases (#5539)
[Markus Stange] Change implementation of withChartViewport (#5542)
[Florian Quèze] A new permalink should be generated and shown after
using the re-upload feature. (#5547)
[Florian Quèze] Show the vertical ruler in the timeline when hovering
the network chart (#5548)
[Markus Stange] Convert the entire codebase to TypeScript (#5549)
[Nazım Can Altınova] Update the yarn.lock file after recent changes
(#5557)
[Markus Stange] Add proper TypeScript coverage for window-navigation.ts
(#5559)
[Markus Stange] Remove leftover $FlowExpectError comments (#5560)
[Markus Stange] Fix Iterator / Iterable confusion (#5561)
[Nazım Can Altınova] Remove the unneeded test-all:ci script (#5566)
[Nazım Can Altınova] Fix a type case inconsistency (#5569)
[Florian Quèze] Make 'yarn lint --fix' work as an alias to 'yarn
lint-fix'. (#5563)
[Ryan Hunt] Don't stringify JSON again in fetchUrlResponse (#5570)
[Nazım Can Altınova] Upgrade ESLint to version 9 (#5567)
[Markus Stange] Simplify Worker setup, and support .json.gz inputs in
symbolicator-cli (#5556)
[Nazım Can Altınova] Add TypeScript coverage to the intersection
observer mock (#5574)
[Markus Stange] Set the preview selection to null when there is no
selection (#5568)
[Markus Stange] Add tests for query-api.ts (#5571)
[Markus Stange] Enable noUnusedParameters and
@typescript-eslint/no-unused-vars and clean up a few more things (#5576)
[Ryan Hunt] Embed iongraph-web and use for iongraph.json source files
(#5577)
[Markus Stange] Remove recursion in filterThreadToSearchString (#5572)


And thanks to our localizers:

be: Mikalai Udodau
de: Michael Köhler
el: Jim Spentzos
en-CA: chutten
en-GB: Paul
es-CL: ravmn
fr: Théo Chevalier
fur: Fabio Tomat
fy-NL: Fjoerfoks
ia: Melo46
it: Francesco Lodolo [:flod]
nl: Mark Heijl
pt-BR: Marcelo Ghelman
ru: Valery Ledovskoy
sv-SE: Luna Jernberg
tr: Rua
tr: Selim Şumlu
uk: Іhor Hordiichuk
zh-CN: Olvcpr423
zh-TW: Pin-guang Chen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants