-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(query-core): allow partial query keys in QueryFilters
#9686
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
base: main
Are you sure you want to change the base?
fix(query-core): allow partial query keys in QueryFilters
#9686
Conversation
…tionFilters` Add a type to match tuple prefixes to match partial query keys in `QueryFilters` and `MutationFilters` Fixes TanStack#9680
WalkthroughUpdated TypeScript typings to allow tuple prefixes for query and mutation keys by adding DropLast and TuplePrefixes utility types; updated QueryFilters.queryKey and MutationFilters.mutationKey types; added type-level tests and a patch changeset. No runtime behavior changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/query-core/src/types.ts
(1 hunks)packages/query-core/src/utils.ts
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-core/src/utils.ts (1)
packages/query-core/src/types.ts (2)
TuplePrefixes
(46-47)MutationKey
(1080-1088)
🔇 Additional comments (1)
packages/query-core/src/utils.ts (1)
36-36
: Blocked by TuplePrefixes recursion bugThis swap to
TuplePrefixes<TQueryKey>
(and the analogous mutation change below) inherits the recursion defect I pointed out on the newTuplePrefixes
helper: for the defaultTQueryKey = QueryKey
we now instantiateTuplePrefixes<ReadonlyArray<unknown>>
, which never terminates. Please see my comment intypes.ts
—we need to add a guard there before these property types can ship.
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.
please add some type tests 🙏
View your CI Pipeline Execution ↗ for commit a348b55 ☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9686 +/- ##
===========================================
+ Coverage 46.38% 60.65% +14.27%
===========================================
Files 214 143 -71
Lines 8488 5726 -2762
Branches 1920 1544 -376
===========================================
- Hits 3937 3473 -464
+ Misses 4108 1953 -2155
+ Partials 443 300 -143 🚀 New features to boost your workflow:
|
…ilters` add tests to verify that the new partial QueryKey matching for `QueryFilters` does match partial query keys, and also doesn't allow invalid query keys.
@coderabbitai review |
✅ Actions performedReview triggered.
|
packages/query-core/src/types.ts
Outdated
export type DropLast<T extends ReadonlyArray<unknown>> = T extends readonly [ | ||
...infer R, | ||
unknown, | ||
] | ||
? R | ||
: never | ||
|
||
export type TuplePrefixes<T extends ReadonlyArray<unknown>> = |
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.
all those types become part of the public interface, I don’t think we want that so please just inline them in utils.ts
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.
That's a good thought! I didn't even think of that. Updated.
prevent tuple helper types from getting exported, and showing up in the public api
🦋 Changeset detectedLatest commit: a348b55 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
5d7869e
to
18a12ca
Compare
🎯 Changes
Add a type to match tuple prefixes to match partial query keys in
QueryFilters
andMutationFilters
Fixes #9680
It's worth noting, there may be a case where it makes sense to turn
QueryFilters
(andMutationFilters
into a discriminated union on theexact
field, so that we only allow partial query filter types in the case whereexact
is false (or unspecified), but it's possible there are cases where one would want partial query key matching even in theexact: true
case, and I'm not sure the added constraint adds much benefit. If you'd like me to add that, let me know.✅ Checklist
pnpm test:pr
.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests
Chores