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

Query returns objects with different identities even though data hasn't changed #4054

Closed
jsslai opened this issue Oct 25, 2018 · 5 comments
Closed
Assignees

Comments

@jsslai
Copy link

jsslai commented Oct 25, 2018

I have a query which returns array of objects. On the UI I'm skipping rendering if single object has same identity oldObject === newObject. Now ApolloClient seems to always return objects with different identities.

Intended outcome:
Query returns objects with same identities because nothing changed.

Actual outcome:
Objects have different identities.

How to reproduce the issue:
Small repro where objects are compared to previous result: https://codesandbox.io/s/4w73lqw097

Versions
apollo-client: ^2.4.3 => 2.4.3
apollo-cache-inmemory: ^1.3.6 => 1.3.6
apollo-link-state: ^0.4.2 => 0.4.2

@benjamn benjamn self-assigned this Oct 26, 2018
@benjamn
Copy link
Member

benjamn commented Oct 26, 2018

This is likely a combination of e66027c (which clones the lastResult to protect against mutation) and the fact that we still pass that previousResult object into InMemoryCache#diff when broadcasting watches here. This logic was updated recently in d6a673f because the === check was not sufficient to establish that nothing has changed, but I can understand why it gets in the way of your design. In all likelihood, the newData.result object is === to your previous result, but the previousResult (which is only isEqual to newData.result) overrides it.

I believe we should be able to remove the previousResult logic altogether, but the last time we tried that some folks reported missing broadcasts. A lot has changed since then, so maybe it's time to try again… or possibly make this behavior configurable/opt-in.

benjamn added a commit that referenced this issue Oct 26, 2018
This commit is a more conservative version of
e66027c

We still need to make a deep clone of observableQuery.lastResult in order
to determine if future results are different (see #3992), but we can
restrict the use of that snapshot to a single method, rather than
replacing observableQuery.lastResult with the snapshot.

Should help with #4054.
@benjamn
Copy link
Member

benjamn commented Oct 26, 2018

@jsslai I can confirm that #4069 will fix your reproduction, though it seems to have broken some tests that expect the query observable next method to be fired a certain number of times. Not sure exactly what changed there, but I will figure it out.

@jsslai
Copy link
Author

jsslai commented Oct 29, 2018

Thanks! I think many React apps do shallow equality checks and this has affect on performance. Missing broadcasts is a problem but I can't think of a use case where app needs to get a broadcast when nothing changed (data, status or error).

benjamn added a commit that referenced this issue Oct 30, 2018
This commit is a more conservative version of
e66027c

We still need to make a deep clone of observableQuery.lastResult in order
to determine if future results are different (see #3992), but we can
restrict the use of that snapshot to a single method, rather than
replacing observableQuery.lastResult with the snapshot.

Should help with #4054.
benjamn added a commit that referenced this issue Oct 30, 2018
This commit is a more conservative version of
e66027c

We still need to make a deep clone of observableQuery.lastResult in order
to determine if future results are different (see #3992), but we can
restrict the use of that snapshot to a single method, rather than
replacing observableQuery.lastResult with the snapshot.

Should help with #4054 and #4031.
@benjamn
Copy link
Member

benjamn commented Oct 30, 2018

This should be fixed by apollo-client@2.4.5!

@benjamn benjamn closed this as completed Oct 30, 2018
@jsslai
Copy link
Author

jsslai commented Oct 31, 2018

It works now. :)

@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants