Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.6.0 Some queries never executing with a conditional skip and overridden fetchPolicy #9635

Closed
Tracked by #9665
dmarkow opened this issue Apr 28, 2022 · 16 comments · Fixed by #9665
Closed
Tracked by #9665

3.6.0 Some queries never executing with a conditional skip and overridden fetchPolicy #9635

dmarkow opened this issue Apr 28, 2022 · 16 comments · Fixed by #9665

Comments

@dmarkow
Copy link
Contributor

dmarkow commented Apr 28, 2022

Intended outcome:

In my example app, clicking a person's name uses setState to store their ID, and then runs a second query to find their country and display it. This second query only executes if a personId is passed in (using skip: !personId).

Actual outcome:

In apollo-client 3.4 and 3.5, this works as expected and displays a country. In 3.6, clicking a person's name does nothing.

How to reproduce the issue:

See https://github.com/dmarkow/apollo-default-options-issue. Open the console and try clicking names. One render completes due to the setState call, however placesLoading from the second query remains false and the query never executes.

This seems to only happen if I have both defaultOptions.watchQuery.fetchPolicy set to anything, and a conditional skip option. If I completely remove the fetchPolicy setting it works, but then I can't control caching on my queries. It also works if I remove the skip: !personId option and change the resolver to return an empty list when no personId is passed in.

Versions

  System:
    OS: macOS 12.3.1
  Binaries:
    Node: 16.14.2 - ~/.volta/tools/image/node/16.14.2/bin/node
    Yarn: 1.22.18 - ~/.volta/tools/image/yarn/1.22.18/bin/yarn
    npm: 8.5.0 - ~/.volta/tools/image/node/16.14.2/bin/npm
  Browsers:
    Chrome: 100.0.4896.127
    Safari: 15.4
  npmPackages:
    @apollo/client: 3.6.0 => 3.6.0
@mversteeg
Copy link

mversteeg commented Apr 28, 2022

I've been wrestling something similar all day.

Basically what I've discovered is that if you update an input to one query from a "then" block of another, the dependent query will never work again (even after force killing the app) until the value of notifyOnNetworkStatusChange is toggled.

I've seen a lot of posters mention that the notifyOnNetworkStatusChange should be true (and that may be the case), but to alleviate the immediate problem of never getting data, I'd start out by trying to invert that flag.

Not sure exactly what the solution is for your particular case (maybe an effect?), but hopefully that toggle operation will unstick you from the "never getting data" problem

@benjamn
Copy link
Member

benjamn commented Apr 28, 2022

@mversteeg Just in case this helps: npm i @apollo/client@next will give you v3.6.1, which may fix some issues related to blocked queries in v3.6.0.

@mversteeg
Copy link

Yeah I've updated to 3.6.1, but am still observing the problematic behavior with the following (simplified) example:

Queries & State:

...
const [givenName, setGivenName] = useState("")
const [familyName, setFamilyName] = useState("")
const [id, setId] = useState("")
const [getDsp, { data: fetchData, loading: fetchLoading, error: fetchError }] = useLazyQuery(queries.dsp, {
    variables: { id },
    notifyOnNetworkStatusChange: true,
  })

const [createDsp, { data: createData, loading: createLoading, error: createError }] = useMutation(mutations.dsp.create, {
    variables: { input: { givenName, familyName } },
  })
...

Create the DSP and update the ID state variable from the "then" block

createDsp({ variables: { input: { givenName, familyName } } })
    .then((ret) => { setId(ret.data.createDsp.id) })

Then, when trying to run the "getDSP" query, it stays in this state:
{data: undefined, loading: true, networkStatus: 2, called: true, previousData: {…}}
until I change something about the query definition itself

I haven't pinned down exactly which changes will cause it to work again, but some of those that have caused the switch for me are:

  • Toggling notifyOnNetworkStatusChange
  • Changing which fields from the response object are destructured. For example, adding "called" to the destructured response above:
const [getDsp, { data: fetchData, loading: fetchLoading, error: fetchError, called }] = useLazyQuery(queries.dsp, {
    variables: { id },
    notifyOnNetworkStatusChange: true,
  })

Hope this helps someone! I think for now I'm just going to be looking for a way to chain requests without using that promise block. Maybe a delay would prevent it if this is caused by a race condition

@FedeBev
Copy link

FedeBev commented Apr 29, 2022

Save here with v3.6.1 and react 18.

In my case the client config is

const apolloClient = new ApolloClient({
  cache,
  connectToDevTools: true,
  link,
  defaultOptions: {
    watchQuery: {
      nextFetchPolicy: "cache-only",
    },
  },
});

and all the query are cancelled (but not the mutations), setting or not the skip field doesn't change anything.
Removing the defaultOptions field from the client config resolves the issue.

@mydesweb
Copy link

v3.6.1 is not working with React Native, queries are not executed. Have to downgrade to v3.5.10

benjamn added a commit that referenced this issue May 2, 2022
benjamn added a commit that referenced this issue May 3, 2022
@saladestomateoignon
Copy link

Hello I had the same issue with v3.6.0, v3.6.1 and v3.6.2. Reverting to v3.5.10 also fixed the issue for me.

benjamn added a commit that referenced this issue May 3, 2022
@benjamn benjamn linked a pull request May 3, 2022 that will close this issue
1 task
@benjamn
Copy link
Member

benjamn commented May 3, 2022

Thanks to @dmarkow's reproduction, I'm confident my PR #9665 will resolve the original issue.

benjamn added a commit that referenced this issue May 3, 2022
@benjamn
Copy link
Member

benjamn commented May 3, 2022

@dmarkow (& others) Can you try npm i @apollo/client@beta to install version 3.7.0-alpha.2? That version includes #9665, which I believe solves this issue, as well as #9666. If we can get some confirmation that these fixes are on the right track, then we can release 3.6.3 with the same changes.

@benjamn benjamn reopened this May 3, 2022
@dmarkow
Copy link
Contributor Author

dmarkow commented May 4, 2022

@benjamn That alpha seems to fix the problem both in my test repo and production app. Thanks!

@saladestomateoignon
Copy link

Hello @benjamn, both v3.7.0-alpha.1 and v3.7.0-alpha.2 did not work for me. I retested v3.5.10 just in case and it worked well (I am on react-native).

My ApolloClient is setup like this:

new ApolloClient({
  cache: new InMemoryCache({
    typePolicies: {
      myResult: {
        fields: {
          myfields: myFieldPolicyObject,
        },
      },
    },
    possibleTypes: myPossibleTypes,
  }),
  link: ApolloLink.from([getAuthStuff(), getHttpStuff()]),
});

@benjamn
Copy link
Member

benjamn commented May 4, 2022

@saladestomateoignon Can you open a new issue with a reproduction? This issue had a reproduction that's fixed now, so it sounds like your issue must be slightly different.

@dyatko
Copy link

dyatko commented May 5, 2022

@benjamn Confirming that 3.7.0-alpha.2 fixed the issue

@Vednus
Copy link

Vednus commented May 5, 2022

@benjamn @saladestomateoignon 3.7.0-alpha.2 fixed the issues for me on react native. I have a custom fetchPolicy and a number of typePolicies and things seem to be working as expected after the update.

@saladestomateoignon
Copy link

@benjamn, I will try to create a reproduction later (when I will have more free time :)).
@Vednus, which version of react-native and react are you using into your project?
(I am using react-native@0.68.0 and react@17.0.2)

Thank you.

@Vednus
Copy link

Vednus commented May 5, 2022

I"m using react@17.0.2 and rn@0.66.4 although I doubt rn is the culprit. Maybe make sure nothing's cached although the js should update on hot reload. Maybe check the @apollo/client package file to verify the version.

@benjamn
Copy link
Member

benjamn commented May 5, 2022

The original reproduction appears to be fixed by @apollo/client@3.6.3, so I'm closing the issue, but please open new issues if you continue to have problems!

@benjamn benjamn closed this as completed May 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.