-
-
Notifications
You must be signed in to change notification settings - Fork 2.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
feat(eslint-plugin-query): add rule to ensure property order of infinite query functions #8072
feat(eslint-plugin-query): add rule to ensure property order of infinite query functions #8072
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 6182304. 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 1 targetSent with 💌 from NxCloud. |
More templates
@tanstack/angular-query-devtools-experimental
@tanstack/eslint-plugin-query
@tanstack/angular-query-experimental
@tanstack/query-async-storage-persister
@tanstack/query-broadcast-client-experimental
@tanstack/query-core
@tanstack/query-devtools
@tanstack/query-persist-client-core
@tanstack/query-sync-storage-persister
@tanstack/react-query-devtools
@tanstack/react-query
@tanstack/react-query-next-experimental
@tanstack/react-query-persist-client
@tanstack/solid-query
@tanstack/solid-query-devtools
@tanstack/solid-query-persist-client
@tanstack/svelte-query
@tanstack/svelte-query-devtools
@tanstack/svelte-query-persist-client
@tanstack/vue-query
@tanstack/vue-query-devtools
commit: |
6736760
to
4084d0b
Compare
...t-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.rule.ts
Outdated
Show resolved
Hide resolved
a08116f
to
4084d0b
Compare
…ite query functions
82228d7
to
a8ad7af
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8072 +/- ##
=========================================
+ Coverage 0 44.97% +44.97%
=========================================
Files 0 199 +199
Lines 0 7362 +7362
Branches 0 1664 +1664
=========================================
+ Hits 0 3311 +3311
- Misses 0 3674 +3674
- Partials 0 377 +377 |
export const checkedProperties = [ | ||
'queryFn', | ||
'getPreviousPageParam', | ||
'getNextPageParam', | ||
] as const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the full order that we must have in the object? Because it shouldn't matter if getNextPageParam
comes before or after getPreviousPageParam
- it only matters that they are after queryFn
. So maybe it needs like a dependency tree:
const propertyDependencies = {
queryFn: ['getPreviousPageParam', 'getNextPageParam']
}
this would define that 'getPreviousPageParam'
and 'getNextPageParam'
depend on queryFn
and thus must be after it. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes ideally a tree like structure would be best. I decided against this in router (similar issue with validateSearch and params.parse) to keep this plugin simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok fine by me for now
Can’t NoInfer<> make type inference work regardless of the order of properties? Or how exactly can the order of properties cause incorrect type inference? |
this is explained here |
I see, thanks for sharing |
No description provided.