-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Changes to context
cause refetches in useQuery
#11835
Comments
bump |
actually you can use a ref to store the context value (always update it synchronously)
this way you can pass through the dynamic context value without introducing side effect |
@jerelmiller does it work for you? |
Hey @kjhuang-db 👋 I don't believe that will work since On top of that React's docs state that you shouldn't read or write refs during render unless its used for initialization as it could make your component behavior unpredicatable. See the caveats section in the docs. I mostly made this note for myself so that I didn't forget about this potential improvement after responding to the thread linked in my message. I and the team think this is fairly surprising behavior in the library and we should address it so that changes to context don't trigger network requests itself. Instead, it should just apply the new context value on the next fetch. This is something our suspense hooks do already, but we should also port that logic over to |
Hey @jerelmiller thanks for the helpful context. I believe just wrapping the ref change with a As of the equality comparison, @kjhuang-db had pointed me earlier that it doesn't do a deep check when the shallow one passes -> https://github.com/benjamn/wryware/blob/main/packages/equality/src/index.ts#L21-L23 which is achieved (in a hack way though) by using the static reference. What I can't say for sure now, without diving deeper into how Totally agreed and +1 that this can create unpredictable behavior (i.e.: it's not very obvious when looking only at the |
@gabrieldutra ah you're right. I missed the small detail that |
yep ..we also misunderstood the "deep equal", turns out it is not that "deep", it is a normal react style equal check |
https://github.com/streamich/react-use/blob/master/src/useLatest.ts , a simple and similar idea to wrap your variable as a ref, if you want |
@jerelmiller I didnt realize you are actually from apollo team member. if so yeah it would be great to address this fine-grained control at apollo level. For now I will use useRef + write it in render (even react is against it) to bypass the equal check |
@jerelmiller is there a timeline where apollo might fix it in the react-hook lib? |
@kjhuang-db we don't have a timeline right now, but I'm willing to bet we'll put this in the next minor version which we'll release in early July. We want to do this in a minor so that we can fix it + provide an option to revert to the old behavior if you rely on this currently. |
Issue Description
Changes to
context
can cause new network requests when updated between renders. This is due to the fact that we do a deep equality check on options and if they change, execute areobserve
. In many cases, this works ok as the applied options should just cause the client to read from the cache again, but when used with afetchPolicy
likeno-cache
, this can cause new refetches. This is especially problematic if you want to useAbortController
to handle cancelling previous fetches.We should also consider expanding this to other options as well. For example, a change to
errorPolicy
should not cause a refetch but rather should be applied to the next fetch. Suspense hooks already have this behavior built-in.We should also consider adding the ability to revert the behavior back to its current form where changes to
context
would cause refetches in case there are users that currently rely on this.Link to Reproduction
https://discord.com/channels/1022972389463687228/1235943877765238847/1235943877765238847
Reproduction Steps
No response
@apollo/client
version3.10.2
The text was updated successfully, but these errors were encountered: