chore: use eslint 9 internally #9119

abrahamguo · 2024-05-18T21:38:43Z

PR Checklist

Addresses an existing open issue: fixes Repo: Use ESLint v9 for our internal linting #9082
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

Use eslint@9 internally.

typescript-eslint · 2024-05-18T21:38:46Z

Thanks for the PR, @abrahamguo!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2024-05-18T21:39:02Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`5ffa053`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6682ae69ef7ba90008f21054
😎 Deploy Preview	https://deploy-preview-9119--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 90 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

nx-cloud · 2024-05-18T21:40:41Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5ffa053. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 31 targets

Sent with 💌 from NxCloud.

bradzacher · 2024-05-22T11:47:41Z

We should delete this CI job

typescript-eslint/.github/workflows/ci.yml

Lines 137 to 154 in 87a0a48

    
           eslint_v9_tests: 
        
             name: Run tests on ESLint v9 
        
             needs: [build] 
        
             runs-on: ubuntu-latest 
        
             steps: 
        
               - name: Checkout 
        
                 uses: actions/checkout@v4 
        
               - name: Install 
        
                 uses: ./.github/actions/prepare-install 
        
                 with: 
        
                   node-version: ${{ env.PRIMARY_NODE_VERSION }} 
        
               - name: Build 
        
                 uses: ./.github/actions/prepare-build 
        
               - run: yarn add eslint@9 
        
               - name: Run tests 
        
                 run: yarn test 
        
                 env: 
        
                   CI: true

have we got an existing integration test to check for regressions on eslint v8?

converting to draft until CI is passing.

abrahamguo · 2024-05-22T12:09:53Z

@bradzacher there are currently no integration or CI tests for eslint 8. I can add one if you want, but do you feel that's necessary since we ask for eslint 9 in both our peerDependencies and devDependencies?

Doesn't seem like anything similar was added for eslint 7 when we moved from eslint 7=>8 (#3737)

Also, seems unrelated to this PR since this is about using eslint 9 internally

bradzacher · 2024-05-22T13:03:56Z

Looks like there's already an integration test for v8 - https://github.com/typescript-eslint/typescript-eslint/tree/main/packages/integration-tests/fixtures/eslint-v8
So nothing to add!

Also, seems unrelated to this PR since this is about using eslint 9 internally

By moving to v9 internally our CI is no longer running against v8 - so we lose the test coverage asserting that we don't regress v8 compat.

But given we have the above integration test - we still have enough coverage.
It's not a super exhaustive test but it'll do (it's the same integration test coverage we maintained when we migrated to v8 internally)

…eslint into eslint-9

mattbrannon · 2024-06-02T09:32:26Z

Just wanted to point out that one of your Netlify jobs has been running for nearly a week.
https://github.com/typescript-eslint/typescript-eslint/pull/9119/checks?check_run_id=25428243220

…eslint into eslint-9

bradzacher · 2024-06-23T07:54:17Z

eslint.config.mjs

      ['eslint-comments']: eslintCommentsPlugin,
      ['eslint-plugin']: eslintPluginPlugin,
-      ['import']: importPlugin,
+      // https://github.com/import-js/eslint-plugin-import/issues/2948
+      ['import']: fixupPluginRules(importPlugin),


we might want to consider switching to eslint-plugin-import-x?

I'd be in support of this as a separate followup yeah.

bradzacher · 2024-06-23T07:55:51Z

eslint.config.mjs

+      ...fixupConfigRules(compat.config(reactPlugin.configs.recommended)),
+      ...fixupConfigRules(compat.config(reactHooksPlugin.configs.recommended)),


it's so weird that we need to fixup the plugin both here and in the plugin def.
seems like quite the flaw in the design of the compat tooling tbh

@bradzacher that's actually not true.

This can be removed if you are OK with me editing the section at the top of this file labeled registering all the plugins up-front, and removing any plugin for which we use a shared config (for example, reactPlugin and reactHooksPlugin).
Shared configs automatically register their corresponding plugin, so it is unnecessary to manually register the plugin.

The flip side, of course, would be if you felt that it's important to preserve a complete up-front list of all plugins used in any part of the config.

If we choose to keep the complete up-front list of plugins, then the reason why we have to use the compat utilities twice, is because the plugins are defined twice (again, redundantly so), and we need to make sure that in both places that each plugin is registered, it is the fixed-up version.

JoshuaKGoldberg

I think we'll need to keep ESLint 8 testing in there for a while - and there is some noise making it harder to review. But encouraging as a start! 🌞

packages/website-eslint/build.ts

.github/workflows/ci.yml

JoshuaKGoldberg · 2024-06-26T03:24:45Z

packages/website/src/theme/MDXComponents/RuleAttributes.tsx

@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/no-unsafe-assignment */


[Non-Actionable] Heh, I wonder how much this will come into conflict with #9339...

packages/website/src/hooks/useClipboard.ts

…eslint into eslint-9

JoshuaKGoldberg

Whoohoo! Thanks!! 🚀

…eslint into eslint-9

packages/eslint-plugin/rules.d.ts

…eslint into eslint-9

packages/eslint-plugin/src/rules/prefer-regexp-exec.ts

packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts

…eslint into eslint-9

abrahamguo · 2024-07-01T13:27:25Z

ESLint 9 displays unused lint suppressions as warnings, not errors (different from ESLint 8, which did not report unused lint suppressions), so there were 3 unused lint suppressions that were displayed in the pipeline but did not block it. I've removed all of those, and provided an explanation for each one.

auvred · 2024-07-01T13:40:19Z

eslint.config.mjs

ESLint 9 displays unused lint suppressions as warnings, not errors, so there were 3 unused lint suppressions that were displayed in the pipeline but did not block it. I've removed all of those, and provided an explanation for each one.

Hmmm, turns out our eslint.config.mjs doesn't have linterOptions: { reportUnusedDisableDirectives: 'error' }. Maybe this is a good chance to turn it on?

This seems a bit suspicious to me though: this option has not been enabled for this repository until now. Maybe there are some considerations in favor of leaving this option disabled?

Just want to clarify that it was the upgrade from ESLint 8 to ESLint 9 that caused it to begin reporting unnecessary lint suppressions.
I feel like it makes sense to keep it at the default value, or change the level from warning to error, but I'm open to further discussion.

Yeah I'd +1 switching to error as a followup. Great find!

auvred

🚀

eslint 9

482dea2

abrahamguo marked this pull request as draft May 18, 2024 22:41

abrahamguo added 2 commits May 21, 2024 20:12

add compat

3623435

add fixup

2bbb91a

abrahamguo force-pushed the eslint-9 branch from c873388 to 2bbb91a Compare May 22, 2024 01:13

abrahamguo added 5 commits May 21, 2024 20:17

fixup react

b3dab19

add more fixups

fa3d79f

add issue comment

ad65eaf

lint and empty types

06bc0d0

fix types

148304f

abrahamguo marked this pull request as ready for review May 22, 2024 01:50

bradzacher marked this pull request as draft May 22, 2024 11:48

bradzacher added this to the 8.0.0 milestone May 22, 2024

abrahamguo added 2 commits May 22, 2024 06:55

remove eslint9 ci test

f665a25

rename

3d2844a

abrahamguo added 3 commits May 22, 2024 07:15

update no-restricted-imports snapshot

e05c917

update build script

7772628

change quote style

1906460

abrahamguo added 3 commits May 23, 2024 07:28

revert renaming

535574b

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

d916841

…eslint into eslint-9

begin migrating config files

d41caf5

Fuzzbender mentioned this pull request Jun 1, 2024

Bump eslint from 8.56.0 to 9.3.0 in /tools/data-handler CyberismoCom/cyberismo#1

Closed

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

2d45bb6

…eslint into eslint-9

bradzacher reviewed Jun 23, 2024

View reviewed changes

JoshuaKGoldberg changed the title ~~feat: use eslint 9 internally~~ chore: use eslint 9 internally Jun 26, 2024

JoshuaKGoldberg requested changes Jun 26, 2024

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jun 26, 2024

abrahamguo added 3 commits June 27, 2024 17:12

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

123e9bb

…eslint into eslint-9

revert style change

fd2973f

add clarifying comment

b08cbd9

abrahamguo requested a review from JoshuaKGoldberg June 28, 2024 11:50

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jun 28, 2024

JoshuaKGoldberg approved these changes Jun 29, 2024

View reviewed changes

JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Jun 29, 2024

abrahamguo added 3 commits July 1, 2024 06:46

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

2e08c7f

…eslint into eslint-9

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

472487b

…eslint into eslint-9

remove unnecessary suppression

3720bd4

abrahamguo commented Jul 1, 2024

View reviewed changes

packages/eslint-plugin/rules.d.ts Show resolved Hide resolved

abrahamguo added 3 commits July 1, 2024 07:30

remove unnecessary suppression

bc22423

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

6cd00f5

…eslint into eslint-9

remove unused suppression

d01d7d6

abrahamguo commented Jul 1, 2024

View reviewed changes

packages/eslint-plugin/src/rules/prefer-regexp-exec.ts Show resolved Hide resolved

abrahamguo commented Jul 1, 2024

View reviewed changes

packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts Show resolved Hide resolved

Merge branch 'v8' of https://github.com/typescript-eslint/typescript-…

5ffa053

…eslint into eslint-9

auvred reviewed Jul 1, 2024

View reviewed changes

JoshuaKGoldberg mentioned this pull request Jul 3, 2024

Repo: Use parserOptions.projectService in main branch linting #8196

Closed

JoshuaKGoldberg requested a review from auvred July 5, 2024 13:17

auvred approved these changes Jul 5, 2024

View reviewed changes

JoshuaKGoldberg merged commit 86811d7 into typescript-eslint:v8 Jul 6, 2024
61 of 62 checks passed

github-actions bot mentioned this pull request Jul 6, 2024

Repo: Use ESLint v9 for our internal linting #9082

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: use eslint 9 internally #9119

chore: use eslint 9 internally #9119

abrahamguo commented May 18, 2024

typescript-eslint bot commented May 18, 2024

netlify bot commented May 18, 2024 •

edited

Loading

nx-cloud bot commented May 18, 2024 •

edited

Loading

bradzacher commented May 22, 2024 •

edited

Loading

abrahamguo commented May 22, 2024

bradzacher commented May 22, 2024

mattbrannon commented Jun 2, 2024

bradzacher Jun 23, 2024

JoshuaKGoldberg Jun 29, 2024

bradzacher Jun 23, 2024

abrahamguo Jun 27, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg Jun 26, 2024

JoshuaKGoldberg left a comment

abrahamguo commented Jul 1, 2024 •

edited

Loading

auvred Jul 1, 2024

abrahamguo Jul 1, 2024

JoshuaKGoldberg Jul 1, 2024

auvred left a comment

		...fixupConfigRules(compat.config(reactPlugin.configs.recommended)),
		...fixupConfigRules(compat.config(reactHooksPlugin.configs.recommended)),

		@@ -1,4 +1,3 @@
		/* eslint-disable @typescript-eslint/no-unsafe-assignment */

chore: use eslint 9 internally #9119

chore: use eslint 9 internally #9119

Conversation

abrahamguo commented May 18, 2024

PR Checklist

Overview

typescript-eslint bot commented May 18, 2024

netlify bot commented May 18, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented May 18, 2024 • edited Loading

☁️ Nx Cloud Report

bradzacher commented May 22, 2024 • edited Loading

abrahamguo commented May 22, 2024

bradzacher commented May 22, 2024

mattbrannon commented Jun 2, 2024

bradzacher Jun 23, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Jun 29, 2024

Choose a reason for hiding this comment

bradzacher Jun 23, 2024

Choose a reason for hiding this comment

abrahamguo Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Jun 26, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

abrahamguo commented Jul 1, 2024 • edited Loading

auvred Jul 1, 2024

Choose a reason for hiding this comment

abrahamguo Jul 1, 2024

Choose a reason for hiding this comment

JoshuaKGoldberg Jul 1, 2024

Choose a reason for hiding this comment

auvred left a comment

Choose a reason for hiding this comment

netlify bot commented May 18, 2024 •

edited

Loading

nx-cloud bot commented May 18, 2024 •

edited

Loading

bradzacher commented May 22, 2024 •

edited

Loading

abrahamguo Jun 27, 2024 •

edited

Loading

abrahamguo commented Jul 1, 2024 •

edited

Loading