previousData is lost when useLazyQuery's executor is called

[tpict]

Intended outcome:
Extraneous calls of the callback returned by useLazyQuery should have no effect.

Actual outcome:
Calling the callback always sets the previousData to undefined. To avoid this behaviour, the user has to keep track of "has the callback been called already?"

How to reproduce the issue:
Here is a codesandbox which should demonstrate the issue in the console tab. There's a variable you can tweak to enable/disable the workaround.

Versions

  System:
    OS: macOS 10.15.7
  Binaries:
    snip
  Browsers:
    Chrome: 87.0.4280.67
    Firefox: 80.0.1
    Safari: 14.0
  npmPackages:
    @apollo/client: ^3.3.1 => 3.3.1
    apollo: ^2.31.1 => 2.31.1

[danieljuhl]

@benjamn any news on this one? Or someway we can help fix this issue? I'll be happy to contribute, if you could point me in the right direction for a fix. According to the following diff, it seems to me, that the test is already covering this, so it seems odd, that previousData is undefined, if the tests actually pass. Or?

https://github.com/apollographql/apollo-client/pull/7082/files#diff-3d31832cd08233c67b25fea350d21eaffd0ca4fbe3c005a33295037ae7066d1b

[Elecweb]

@benjamn

I've take a look at #6603 and look like this is intentional.

I think this doesn't different than useQuery, It should preserve previousData. For example, we can use previousData for checking that if it's is first fetch or not (for show different loading UI for initial and subsequent call). We can't use called because it's set to true immediately even though loading is still true

I'm also interested to see a use case where reset previousData to undefined can be useful.

I or @dahjelle could help to contribute this.

[benjamn]

@Elecweb @dahjelle We'd be happy to review a PR, or even just a failing regression test submitted as a draft PR.

As a small note: I would recommend starting from the release-3.4 branch rather than main, since AC3.4 is in the final stages of prerelease testing.

[brainkim]

Hmmm. So I looked into this a little bit and I’m a bit confused by the useLazyQuery() API. There are two functions you can call to “refetch” the query. The first is the execute() function (the first element of the tuple passed by the useLazyQuery() hook. Calling the execute() function a second time seems to cause both the data and previousData to be blown away. In essence, it seems to be starting the query from scratch. The second is the refetch() function, (returned as a property of the second element of the tuple). Calling the refetch() function seems to do what the people in this issue are looking for, which is preserving the current data as previousData when variables change.

const [execute, {data, previousData, refetch} = useLazyQuery(MY_QUERY);
// in some callback
onFirstEvent(() => {
  // Calling the execute function
  execute({variables: {id: 1}});
});

onSecondEvent(() => {
  // Calling the execute function
  execute({variables: {id: 2}});

  // Calling the refetch function
  refetch({id: 2});
});

Note that the call signatures of the execute() function and the refetch() function are slightly different. The execute() function takes an object with variables, the refetch() function takes the variables directly. Also, I think the refetch() function will be undefined until execute() is called.

This is a lot of words to say that you can get the desired behavior if you call execute() the first time, and refetch() for subsequent fetches. I don’t think this behavior is in any ways ideal, insofar as it’s very confusing as to why there might be two functions that both do refetching. I will continue to investigate this.

We’ve been thinking in AC4 of deprecating useLazyQuery(), insofar as it might just be easier to call the core client APIs directly in a useEffect when you need fine-grained control over when a query is made. Would that make people in this issue upset?

I have a unit test on this branch, which shows that this is the behavior.

[tpict]

We’ve been thinking in AC4 of deprecating useLazyQuery(), insofar as it might just be easier to call the core client APIs directly in a useEffect when you need fine-grained control over when a query is made. Would that make people in this issue upset?

My initial thought is "no", but could you give an example of what the useEffect callback would look like? I agree that the distinction between execute and refetch has always felt like a wart, but I get the impression that useLazyQuery is very widely used, so its removal might be upsetting to people if the replacement is much less ergonomic. Thanks for looking into this!

[hwillson]

This should now be resolved in @apollo/[email protected]. Let us know if you notice any issues. Thanks!