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

feat!(eslint-plugin-query): Migrate to ESLint v9 with Flat Config Support #7253

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

davidjbng
Copy link
Contributor

@davidjbng davidjbng commented Apr 9, 2024

This PR is a starting point to add support for the newly released ESLint v9 as well as exposing a new flat/recommended to be used in eslint.config.js.

Edit: I noticed the separate flat/recommended config may not be necessary given it exports the same rules as the recommended config anyway.

I confirmed this works on my project testing only with eslint@9, but haven't tested the old recommended config, neither did I test with eslint@8.

Resources

I used the following resources to find out which changes to make

To Be Discussed

  • Support for the Flat Config could be its own PR if the changes to support ESLint v9 turn out to be breaking
  • Use the existing 'recommended' config for Flat Config support if possible. *Rule definitions requirements seem to have changed so having a separate flat config is required to support both formats.
  • Find a good way to fully test the exported config. Including the plugin object itself as required by flat config adds a circular reference with a lot of noise in the snapshot
  • See if config types could be improved
  • I had some trouble using the build locally and had to change the exports in packages.json from ./dist/esm/index.js to ./dist/esm/src/index.js to use the package installed from a directory. Probably not an issue if it works in ci, but wanted to mention it just in case.

TODO

  • Update Docs
  • Test with ESLint v8
  • Update examples/react/shadow-dom and examples/react/basic-typescript

Related Issues

Copy link

vercel bot commented Apr 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) Visit Preview Apr 10, 2024 9:38am

Copy link

nx-cloud bot commented Apr 9, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cf11b6a. 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


🟥 Failed Commands
nx affected --targets=test:format,test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build,test:attw --parallel=3

Sent with 💌 from NxCloud.

@davidjbng davidjbng changed the title Migrate to ESLint v9 with Flat Config Support feat!(eslint-plugin-query): Migrate to ESLint v9 with Flat Config Support Apr 9, 2024
@davidjbng
Copy link
Contributor Author

Also note how eslint-plugin-react-hooks will handle v9 and flat config support in

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 10, 2024

Thanks for this. We'd definitely need to update docs and show how to use the plugin both for flat config and the v8 syntax.

@TkDodo TkDodo requested a review from Newbie012 April 10, 2024 07:08
@TkDodo
Copy link
Collaborator

TkDodo commented Apr 10, 2024

lockfile seems to be outdated:

 ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with packages/eslint-plugin-query/package.json

@davidjbng
Copy link
Contributor Author

lockfile seems to be outdated:

 ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with packages/eslint-plugin-query/package.json

The root package.json requires all packages to use the same ESLint version.

query/package.json

Lines 83 to 89 in fca5e7d

"pnpm": {
"overrides": {
"@typescript-eslint/eslint-plugin": "$@typescript-eslint/eslint-plugin",
"@typescript-eslint/parser": "$@typescript-eslint/parser",
"eslint": "$eslint"
}
}

Besides the ESLint Plugin - which I updated to v9 - eslint v8 is currently depended on by examples/react/shadow-dom and examples/react/basic-typescript which need to be fixed. Updating the examples to Flat Config is blocked by the upstream react plugin issues though.

Copy link
Collaborator

@Newbie012 Newbie012 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cf11b6a:

Sandbox Source
@tanstack/query-example-angular-basic Configuration
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

@davidjbng
Copy link
Contributor Author

I think this has to wait until it is offically supported by typescript-eslint which is tracked here

The tests are still failing because of differences how rule definitions work for the flat config format, but @typescript-eslint/rule-tester isn't up to date to report those errors in typescript. Rather you get runtime errors directly from eslint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants