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

Settle Feuds with Data #6777

Merged

Conversation

danReynolds
Copy link
Contributor

Ran into a scenario as described by @benjamn here: https://github.com/apollographql/apollo-client/blob/master/src/core/QueryInfo.ts#L237

where queries were feuding over a piece of data in the cache, causing an infinite loop of network requests. Normally this mechanism would have prevented them from continuing on in that loop because they were returning the same network result data every time, however, our Apollo server has tracing extensions applied as described here: https://github.com/apollographql/apollo-tracing

which causes this check to fail:
https://github.com/apollographql/apollo-client/blob/master/src/core/QueryInfo.ts#L238

since the result objects include the extensions and the extensions include unique time sensitive data like query durations and start/end time. The simple fix is to check for equality of the data properties of the results rather than the entire result objects. If that has consequences I'm not aware of, then it might be better to check equality of the objects, omitting the extensions property.

@benjamn benjamn added this to the Post 3.0 milestone Aug 5, 2020
This syntax will be great once it's natively supported, but TypeScript
currently compiles it to a much larger expression than we need.
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea @danReynolds! It makes perfect sense to me to check only result.data, since that's all we're writing into the cache. Other result properties shouldn't be contributing to the problem that this logic is trying to prevent.

@benjamn benjamn merged commit 759619b into apollographql:master Aug 6, 2020
@xfournet
Copy link

Thanks @danReynolds! i was also fighting with the same problem and come to the same conclusion. Happy to see the fix is already available

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants