Variables not updated when refetch with useQuery() + skip: true

[chenesan]

Recently our teams just migrated apollo-client from v2 to v3, and found out a behavior change:

In our codebase we call useQuery() with skip: true, updating the variables during rerender, and call the returned refetch() to fire the query, for example:

function Foo() {
  // we update `count` in other place and trigger variables change.
  const [count, setCount] = useState(0);
  const { refetch, data } = useQuery({
    skip: true,
    variables: { count },
  });

  // skip other code...
  
  // Fire refetch on user click button(just for example)
  return <button onClick={() => refetch()} />
}

We do useQuery() + skip: true + refetch because useLazyQuery() returns void instead of promise. We have used this workaround for a long time.

Then we found out that, in v3 (3.4.10), when refetch(), the request will not bring the latest variables but the initial variables. In above case, the count in variables will always be 0.

In v2, the variables will always be updated, which is the behavior we expected.

Related PR

The behavior change seems start from #6752. It prevents network request when options update with skip: true, but it also prevents options update.

This comment shows that a portion of apollo-client users do use useQuery() + skip: true + refetch() as a substitution of useLazyQuery(). For us, this behaivor change is an implicit breaking change and cause unexpected bug during the migration from v2 -> v3(we spend one month discovering this issue after finishing v3 migration). I hope we can fix this behavior change so it would be less painful for people migrating to v3.

Intended outcome:

When call refetch() with useQuery() + skip: true, the query options should be updated.

Actual outcome:

When call refetch() with useQuery() + skip: true, the query options are not updated.

How to reproduce the issue:

codesandbox: https://codesandbox.io/s/recursing-goodall-mcd76

The request will fired when renderTimes > 3, Notice that the fired request will always bring outdated variables limit: 1 instead of updating limit with renderTimes.

Versions

System:
    OS: macOS 10.15.3
  Binaries:
    Node: 12.16.3 - ~/.nvm/versions/node/v12.16.3/bin/node
    Yarn: 1.22.15 - ~/.yvm/shim/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v12.16.3/bin/npm
  Browsers:
    Chrome: 94.0.4606.81
    Firefox: 89.0
    Safari: 13.0.5
  npmPackages:
    @apollo/client: ^3.4.10 => 3.4.10

[brainkim]

@chenesan Sorry for the delayed response! So looking at your repro on Code Sandbox, I’m wondering why the hook which calls setRenderTimes() does not have a dependency array? Also note that the refetch() function can take new variables. Finally, I was wondering if you could try this in @apollo/client@beta? There’s been a significant refactoring of the React layer, and this problem might be fixed there.

[chenesan]

@brainkim
Thanks for the response!

So looking at your repro on Code Sandbox, I’m wondering why the hook which calls setRenderTimes() does not have a dependency array?

Just rewrite the effect with setInterval, sorry for misleading 🙇 .
Here I just want to demo that the renderTimes changes in each render and update the variables in query options, however the variables sent by refetch() is still initial 1(initial value of renderTimes), not renderTimes.

Also note that the refetch() function can take new variables.

Yes, this can resolve the problem. But, before migrate to v3 we don't need to pass new variable and apollo-client can take the newest variables from options to refetch even though the query is skipped. After #6752 if we don't pass new variables it will ignore the variables passed to skipped query instead, which is a breaking change I think? For this behavior change, we have to check if we call refetch without new variables all over our codebase 😢

Finally, I was wondering if you could try this in @apollo/client@beta? There’s been a significant refactoring of the React layer, and this problem might be fixed there.

Tried 3.5.0-beta.18 in codesandbox and still the same.

[jpvajda]

@chenesan Hello! it's been a while but I'm curious if this is still an issue for you and if you can try out 3.6.7. We've recently done a lot of improvements to useQuery and some other community members have posted a few issues with @skip So I'd be curious to know what your experience is now.

#9673
#9765

[chenesan]

it's been a while but I'm curious if this is still an issue for you and if you can try out 3.6.7

I can try this next week.

[chenesan]

Tried 3.6.9 and looks good now! thanks @jpvajda

[jpvajda]

@chenesan Awesome! We appreciate the confirmation.