Refactor useQuery using InternalState class to simplify reasoning about state
#9459
Conversation
…the component re-renders
I will revisit this approach to prevent repeated updates of unchanging result properties (like obsQueryFields).
Removing result.partial when it's truthy defeats the whole purpose of this (questionably necessary) field.
Follow-up/refinement to PR #9210. Now that the createWatchQueryOptions function no longer defaults watchQueryOptions.fetchPolicy to "cache-first" unconditionally (instead defaulting to defaultOptions.watchQuery.fetchPolicy if defined), we can let the merging of defaultOptions happen as it normally does, when client.watchQuery(watchQueryOptions) is called later.
Using options as a useMemo dependency only happens to work here when options is undefined. If options is an object created fresh every time useQuery is called (as it often is), the useMemo version of this code would produce a new watchQueryOptions object on every call.
This handful of test changes all seem defensible to me.
benjamn
force-pushed
the
useQuery-internal-state-ref
branch
from
c9464c2 to
d280d73
Compare
|
Hi, I've had a chance to look at this. It's good to see the original issue in #9204 is resolved. I do still see a difference in behavior in this PR vs pre-3.5.x. Please see this PR: #9508. I've added two tests that test the exact frame output of Similar tests could be added for Is frame-testing something that you are interested in? Are you willing to incorporate the frame-tests to document the behavior of |
|
@FritsvanCampen Thanks for the review! I'd like to hear more about frame testing, but let's continue in #9508? In the meantime, I'll merge this PR and release another beta, so everyone can easily test it. |
This work began as a solution to issue #9204, incorporating the tests contributed by @jamesopti and @FritsvanCampen in #9407 and #9412. It snowballed into a more general refactoring of the
useQueryhook, somewhat resembling theInternalStateapproach I used foruseBackgroundQueryin #8783.I feel like this approach has been productive so far, but before I spend more time on it, I wanted to share what I'm thinking (especially with @brainkim and @hwillson) in this draft PR.
I don't know if reviewing each commit is worthwhile, since some of the earlier commits are undone later, but I tried to do things incrementally, keeping tests passing with each commit, in case that's a helpful narrative.
This PR targets
release-3.6, which seems appropriate for a refactoring of this size, but I am hopeful we can find a smaller/safer subset of the changes that we could ship in a 3.5.x patch release, so #9204 and related issues can be solved without waiting for v3.6.Because this PR interacts so much with
useQueryoptions handling, I've been holding off on #9223 and #9222 and some other issues until the dust has settled here.