3.5.0 render frames of useQuery changed when variables change

[FritsvanCampen]

In 3.5.0 the implementation of useQuery has been completely rewritten. Although no tests fail, the render behavior changed. There are some concerns with that:

1. It's currently unclear what the correct behavior is.
2. There are no tests (zero) covering exact render frame output.

Look at this test for example: "useQuery hook:General Use:should work with variables": https://github.com/apollographql/apollo-client/blob/main/src/react/hooks/__tests__/useQuery.test.tsx#L137

If you look at result.all at the end of the test, the frames will have values of loading and data like:

{ loading: true, hasData: false },
{ loading: false, hasData: true },
{ loading: false, hasData: true }, // this frame is duplicated and was not present in <3.5.0
{ loading: true, hasData: false },
{ loading: false, hasData: true }

Here is a rundown of why each frame happens:

1. Initial state, loading is true, we have no data.
2. Data becomes available, loading is false.
3. Variables change (render from outside)
4. React to variable change by setting loading to true to data to empty.
5. New data becomes available.

In <3.5.0 frames 3 and 4 were combined and they theoretically can be again. Whether this should be changed comes down to ideology perhaps. Should a hook be permitted to add theoretically unnecessary render frames? Adding one extra frame in 4 frames isn't going to break the bank, but as it is there are no tests guaranteeing any kind of reasonable restriction. The code could add 10 extra render frames and all tests would still pass. I think that's a problem.

So I think we need to solve 2 problems:

1. Start testing for exact render frames.
2. Remove the unneeded frame.

We can achieve 1 by asserting the value of result.all, counting frames, and ensuring things happen exactly as we expect.

We can achieve 2 by refactoring the internals. result is a state internally. Changing it to a refObject allows for more fine-grained render-frame behavior.

Is this a direction the Apollo team is willing to go? Or should users of this library figure out a way to deal with 'extra' render frames, and potentially unbound and undocumented changes to the render-frames?

[FritsvanCampen]

The effect on useLazyQuery is even more dramatic, adding several unneeded frames.

[dmarkow]

Possibly related to this: #9135 (comment) -- 3.5.6 fixed the variables property being stale, but it's still including this unnecessary render loop with loading: false and the old data. We do some detection on loading being false and data being present that is breaking some redirects and pieces of our UI.

Put another way: with this new render added in, if we update our query variables, how is our app supposed to know that { loading: false, variables: { new_variable: "blah blah blah"}, data } is actually returning stale data based on the previous variables?

[haizz]

A render frame with old data causes scrolling restoration problems in SPA with navigation based on browser history API. The issue leads to really annoying user experience.

[bengotow]

Hey folks - I think this may be related to the bug I filed yesterday with a repro case: #9333. I tested using 3.5.7 and also observed the variables property is immediately updated but the data is still old.

[bignimbus]

Hi all, I'm doing some housekeeping and believe this issue was resolved by #9599. I'll close this for now but please reach out if you need more support 🙏🏻

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.