apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.38k stars 2.66k forks source link

data is not available anymore in the onCompleted callback #11996

Closed franbach closed 3 months ago

franbach commented 3 months ago

Hello friends!

I'm trying to update to the latest Apollo version but I noticed a behavior that was not present in the version 3.10.8.

I believe this is not an ideal pattern but it's currently being used in the codebase I'm working on.

Works on 3.10.8

const { data } = useQuery<MyQueryType, MyQueryVariablesType>(MY_QUERY, {
  variables: { ... },
  onCompleted: () => {
    // do something with data here
    process(data)
  }
})

On the latest version, data is now undefined or at least after 3.10.8. I've read the changelog but it doesn't seem to mention this behavior change. I believe the right approach would be to just use the data that onCompleted gives us in return, like...

onCompleted: (data) => process(data);

I just want to understand what changed so I can explain it to my peers.

I appreciate any feedback!

Thanks, everyone :)

alessbell commented 3 months ago

Hi @franbach 👋 Thanks for reporting this.

On the latest version, data is now undefined or at least after 3.10.8

My intuition is that some of the refactoring work that @phryneas did to make useQuery compliant with the Rules of React and React Compiler (https://github.com/apollographql/apollo-client/pull/11869) changed the timing so that when onCompleted is called the first time, the hook hasn't already triggered a re-render with data. That this used to work is incidental and not behavior that was documented or supported, as data was meant to be consumed via the callback function's data parameter, as you noted.

I believe the right approach would be to just use the data that onCompleted gives us in return, like...

Yes, though in general we advise against doing any "post-processing" of data in onCompleted, since this can result in component state getting out of sync with the data returned from the hook. For example, as of v3.8 we fixed a bug that was causing onCompleted to execute on cache writes without network requests originating from the hook.

Since onCompleted doesn't necessarily run every time the hook will re-render with new data, we advise creating a "derived value" that reads from data directly instead:

const { data } = useQuery<MyQueryType, MyQueryVariablesType>(MY_QUERY, {
  variables: { ... }
});

// you can use memoization to avoid re-computing `processedData` if `data` hasn't changed
const processedData = useMemo(() => process(data), [data]);

I hope that's helpful! Let me know if you have any other questions, otherwise I'll go ahead and close this out :)

franbach commented 3 months ago

Awesome! yes you can close it, thank you so much @alessbell :)

github-actions[bot] commented 3 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

github-actions[bot] commented 2 months ago

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.