Skip to content

Conversation

@iammminzzy
Copy link
Contributor

@iammminzzy iammminzzy commented Dec 31, 2023

Resolves #5279 (comment).

I took the existing prefer-query-object-syntax rule and modified it to look for deprecated callbacks.

@vercel
Copy link

vercel bot commented Dec 31, 2023

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 Jan 5, 2024 8:51pm

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 31, 2023

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 09c56ae:

Sandbox Source
@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

@iammminzzy iammminzzy force-pushed the eslint-plugin/callbacks branch 4 times, most recently from 0fa4fd5 to e51074b Compare December 31, 2023 16:38
@iammminzzy iammminzzy force-pushed the eslint-plugin/callbacks branch from e51074b to ac7232f Compare December 31, 2023 16:51
@nx-cloud
Copy link

nx-cloud bot commented Jan 2, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 09c56ae. 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 2 targets

Sent with 💌 from NxCloud.

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2ec1979) 96.46% compared to head (09c56ae) 96.19%.
Report is 772 commits behind head on v4.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##               v4    #6622      +/-   ##
==========================================
- Coverage   96.46%   96.19%   -0.27%     
==========================================
  Files          45       26      -19     
  Lines        2261      946    -1315     
  Branches      641      279     -362     
==========================================
- Hits         2181      910    -1271     
+ Misses         77       30      -47     
- Partials        3        6       +3     

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

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 2, 2024

Thanks, I think we can ship this, but tbh, I don't really think it's worth it. The overlap of:

  • people knowing about the eslint-plugin
  • updating the eslint-plugin to the latest version
  • still being on v4 of react-query
  • knowing about and using the callbacks
  • not already knowing that they are deprecated

is probably very close to zero

@Newbie012 what do you think? At least we would also need to update the docs please, and we should widen the rule to be no-deprecated-properties and look for all deprecated ones. But I really don't think it's worth the effort ...

@NMinhNguyen
Copy link
Contributor

NMinhNguyen commented Jan 2, 2024

Thanks, I think we can ship this, but tbh, I don't really think it's worth it. The overlap of:

  • people knowing about the eslint-plugin
  • updating the eslint-plugin to the latest version
  • still being on v4 of react-query
  • knowing about and using the callbacks
  • not already knowing that they are deprecated

is probably very close to zero

Just to share my perspective, the rule in its current form is still incredibly helpful. At work we have a big monorepo with more than 100 apps, and although we are aware of the deprecation, it’s not easy to upgrade to v5 precisely because of the deprecated callbacks. With this ESLint rule, we can gradually turn on the rule. But with TypeScript, it’s kinda an all or nothing approach - you can’t really upgrade package by package as well as prevent hundreds of developers from using the callbacks in new code.

@Newbie012
Copy link
Collaborator

I think @NMinhNguyen has a point.

At work, we also didn't upgrade to v5 (due to other reasons). With ESLint, you can enforce rules incrementally, unlike upgrading to v5 which is all or nothing.

LGTM.

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 2, 2024

Understood. I would still like these 3 things please:

  • docs
  • rename to no-deprecated-options
  • widen the rule to include other options that are deprecated in v4

@NMinhNguyen
Copy link
Contributor

widen the rule to include other options that are deprecated in v4

Could you please help enumerate what these are? Presumably there is a v4 alternative for each one of them?

@TkDodo
Copy link
Collaborator

TkDodo commented Jan 2, 2024

widen the rule to include other options that are deprecated in v4

Could you please help enumerate what these are? Presumably there is a v4 alternative for each one of them?

useQuery options

  • the isDataEqual option is deprecated
  • onSuccess, onError and onSettled are deprecated

others

  • remove returned from useQuery is deprecated
  • the contextSharing option on the QueryClientProvider is deprecated
  • the logger option on QueryClient constructor is deprecated

it's fine if we just do the useQuery options, or, if you want to go deeper, feel free to do the others as well.

@iammminzzy
Copy link
Contributor Author

Thank you so much for the review and suggestions, @TkDodo!

it's fine if we just do the useQuery options

I decided to keep it simple for now and just handle the deprecated callbacks as well as isDataEqual.

Additionally, the rule would previously highlight the entire useQuery() call, but now it should only target individual options:

before

image

after

image

@iammminzzy iammminzzy changed the title feat(eslint-plugin): add no-callbacks rule feat(eslint-plugin): add no-deprecated-options rule Jan 5, 2024
@iammminzzy iammminzzy force-pushed the eslint-plugin/callbacks branch from 6c3c9cf to 69a0c87 Compare January 5, 2024 19:56
@TkDodo TkDodo merged commit 84e8ae5 into TanStack:v4 Jan 5, 2024
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.

5 participants