-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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: typed query filters #8670
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 3401096.
☁️ Nx Cloud last updated this comment at |
Sizes for commit 3401096:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8670 +/- ##
===========================================
+ Coverage 46.28% 63.05% +16.77%
===========================================
Files 199 135 -64
Lines 7549 4851 -2698
Branches 1732 1371 -361
===========================================
- Hits 3494 3059 -435
+ Misses 3675 1547 -2128
+ Partials 380 245 -135 |
invalidateQueries< | ||
TQueryFnData = unknown, | ||
TError = DefaultError, | ||
TTaggedQueryKey extends QueryKey = QueryKey, | ||
TInferredQueryFnData = InferDataFromTag<TQueryFnData, TTaggedQueryKey>, | ||
TInferredError = InferErrorFromTag<TError, TTaggedQueryKey>, | ||
>( | ||
filters?: InvalidateQueryFilters< | ||
TInferredQueryFnData, | ||
TInferredError, | ||
TInferredQueryFnData, | ||
TTaggedQueryKey | ||
>, | ||
options?: MaybeRefDeep<InvalidateOptions>, | ||
): Promise<void> | ||
invalidateQueries( |
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.
@DamianOsipiuk I don’t really know what I’m doing here 😂 but I’m getting type errors if I don’t add an overload that has the same signature as the base version 🤔 . Is this correct ?
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.
To make TS happy i guess you would need something like below.
And yes, you need to include both base and MaybeRefDeep
if you are using TS inference.
invalidateQueries<
TQueryFnData = unknown,
TError = DefaultError,
TTaggedQueryKey extends QueryKey = QueryKey,
TInferredQueryFnData = InferDataFromTag<TQueryFnData, TTaggedQueryKey>,
TInferredError = InferErrorFromTag<TError, TTaggedQueryKey>,
>(
filters: InvalidateQueryFilters<
TInferredQueryFnData,
TInferredError,
TInferredQueryFnData,
TTaggedQueryKey
>,
options: InvalidateOptions,
): Promise<void>
invalidateQueries<
TQueryFnData = unknown,
TError = DefaultError,
TTaggedQueryKey extends QueryKey = QueryKey,
TInferredQueryFnData = InferDataFromTag<TQueryFnData, TTaggedQueryKey>,
TInferredError = InferErrorFromTag<TError, TTaggedQueryKey>,
>(
filters: MaybeRefDeep<
InvalidateQueryFilters<
TInferredQueryFnData,
TInferredError,
TInferredQueryFnData,
TTaggedQueryKey
>
> = {},
options: MaybeRefDeep<InvalidateOptions> = {},
): Promise<void> {
const filtersCloned = cloneDeepUnref(filters)
const optionsCloned = cloneDeepUnref(options)
super.invalidateQueries(
{ ...filtersCloned, refetchType: 'none' },
optionsCloned,
)
if (filtersCloned.refetchType === 'none') {
return Promise.resolve()
}
const refetchFilters: RefetchQueryFilters<
TInferredQueryFnData,
TInferredError,
TInferredQueryFnData,
TTaggedQueryKey
> = {
...filtersCloned,
type: filtersCloned.refetchType ?? filtersCloned.type ?? 'active',
}
// (dosipiuk): We need to delay `refetchQueries` execution to next macro task for all reactive values to be updated.
// This ensures that `context` in `queryFn` while `invalidating` along reactive variable change has correct
return nextTick().then(() => {
return super.refetchQueries(refetchFilters, optionsCloned)
})
}
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.
Can you explain why the versions with MaybeDeepRef
are optional, but without it, the params are required ? I think they should be optional in both overloads ...
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.
like so?
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.
Oh yeah, overload should include optionals.
MaybeRef
just does not like | undefined
union, so to get rid of it and properly infer types, it's instead defaulted to empty object = {}
Looks good 👍
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.
@DamianOsipiuk I don’t understand where this type error now comes from:
../vue-query/src/__tests__/queryClient.test.ts(198,11): error TS2322: Type 'Ref<boolean, boolean>' is not assignable to type 'boolean | undefined'.
{ cancelRefetch: ref(false) }, |
I had to change options
again: 3401096
No description provided.