3.5 hotfixes #9144
3.5 hotfixes #9144
Conversation
brainkim
force-pushed
the
brian-3.5-triage
branch
from
665e83d
to
29bb0db
Compare
… loops by useQuery in v3.5.x" in a test. #9135 The test seems to be passing so we’re probably going to have to go in blind.
There are instances in Apollo Studio where the execute function from useMutation is placed in a useEffect dependency array for some reason. The fact that the callback can change seems to cause React-185s. This change makes the execute function the same for the lifetime of the component. This also incidentally seems to fix #9111 because we’re now updating the ref directly in the render function. I dunno anymore.
brainkim
force-pushed
the
brian-3.5-triage
branch
from
29bb0db
to
ab926d7
Compare
An attempt to fix #9135. I was not able to create a unit test to reproduce the bug. It seems like we just can’t rely on `useEffect()` for certain parts of the hooks. The tests that changed just seem to be about random legacy component timings.
brainkim
force-pushed
the
brian-3.5-triage
branch
from
4a088e8
to
ec8a5fb
Compare
| const client = new ApolloClient({ | ||
| defaultOptions: { | ||
| watchQuery: { | ||
| pollInterval: 10, |
There was a problem hiding this comment.
Is there any risk this client will keep polling after the test finishes? Do we need to call client.stop() or something like that?
There was a problem hiding this comment.
I’m pretty sure component teardown will clear the polling interval, which each test does?
| { | ||
| Object.assign(ref.current, { client, options, mutation }); | ||
| } |
There was a problem hiding this comment.
Ah I’ve just developed a weird habit where I’m putting bad-boy rendering “side-effects” in a single block. I can remove the block if you’d prefer.
|
|
||
| // We assign options during rendering as a guard to make sure that | ||
| // callbacks like onCompleted and onError are not stale. | ||
| Object.assign(ref.current, { options }); |
There was a problem hiding this comment.
Should this be Object.assign(ref.current, { options: obsQuery.options }), or are those objects guaranteed to be === here?
There was a problem hiding this comment.
The options here are the ones passed into the call. obsQuery.options is not guaranteed to be up-to-date. This was preemptively done so that onError and onCompleted are never stale.
Review by commit.
Moves a bunch of logic directly in the render function where side effects shouldn’t be done, but we’re doing it because
useEffect()is just not to be trusted.Fixes #9111, #9135