-
-
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
Function passed to notifyOnChangeProps
cannot return undefined
#7426
Labels
Comments
The resolution should be to just update query/packages/query-core/src/types.ts Line 147 in 1d60d44
to
Happy to open a PR if my understanding described above is correct. |
Yes you are right. It will already work at runtime because if
Happy to accept a PR to adapt the types 👍 |
winghouchan
added a commit
to winghouchan/react-query
that referenced
this issue
May 16, 2024
…option to return `undefined` `undefined` is a valid return value for the function value of `notifyOnChangeProps`. See linked issue for more details. Fixes TanStack#7426.
winghouchan
added a commit
to winghouchan/react-query
that referenced
this issue
Jul 17, 2024
…option to return `undefined` `undefined` is a valid return value for the function value of `notifyOnChangeProps`. See linked issue for more details. Fixes TanStack#7426.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Background
notifyOnChangeProps
is a query option that determines if the query should notify listeners if query properties change. It has the following type:query/packages/query-core/src/types.ts
Lines 144 to 148 in 1d60d44
Depending on the value set, it has the following behaviours:
undefined
: The query will track which properties are accessed and only notify listeners if those properties change. This is the default option.string[]
: The query will notify listeners if any of the properties in the array change.'all'
: The query will notify listeners if any properties change.It can also be set with a function with the signature
() => string[] | 'all'
. The query will run the function and follow the behaviour listed above that matches the returned value.With the available non-function options of
undefined
,string[]
, or'all'
, it follows that the function should also be able to returnundefined
. However, the current type signature does not allow it.Real world example
One real world example of the function passed to
notifyOnChangeProps
returningundefined
comes from React Query's documentation itself, for disabling re-renders on out of focus screens on React Native. See this TypeScript Playground for a reproduction.Impact
Allowing the function passed to
notifyOnChangeProps
to returnundefined
should not have any negative impact. At runtime, the query observer sets the value ofnotifyOnChangeProps
or the value of the call tonotifyOnChangeProps
if it was a function to the same variable. This means ifundefined
is a valid value fornotifyOnChangeProps
then a call tonotifyOnChangeProps
should also be allowed to beundefined
. See:query/packages/query-core/src/queryObserver.ts
Lines 612 to 616 in 1d60d44
Your minimal, reproducible example
https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAbzgVwM4FMCKz1QJ5wC+cAZlBCHAOQACMAhgHaoMDGA1gPRTr2swBaAI458VAFDi0WUXgAUCcXGVwRuPADFGALjj1UeRqzhyAlHAC8APjgwoOADRKVa-AGl0eXQG0qrvB54VAC6TipwjBAwwCR4APKMAMIAFkwA5ugACuRgqLpmljbIjAAm6CTAjOglToSmQA
Steps to reproduce
Set
notifyOnChangeProps
query option to a function that (could) returnundefined
.Expected behavior
No type error should occur.
How often does this bug happen?
Every time
Screenshots or Videos
No response
Platform
N/A
Tanstack Query adapter
None
TanStack Query version
5.29.2
TypeScript version
5.4.5
Additional context
No response
The text was updated successfully, but these errors were encountered: