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

Call iterateObserversSafely if vars change between calls to observer.next #10143

Merged
merged 5 commits into from
Oct 17, 2022

Conversation

alessbell
Copy link
Contributor

@alessbell alessbell commented Sep 28, 2022

Closes #10105.

This PR updates the condition inside of src/core/ObservableQuery.ts's reportResult to check whether an observable query's variables have changed in between calls to observer.next. This resolves the linked issue: when loading remains true after observer.refetch is called multiple times with different variables when the same data are returned.

Previously, this would result in the lastError || this.isDifferentFromLastResult(result) check returning false, and iterateObserversSafely would not execute with the new result containing loading: false.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@alessbell alessbell requested a review from benjamn September 28, 2022 22:10
@alessbell alessbell force-pushed the issue-10105-loading-stays-true branch from 3e3d33f to 31cecd6 Compare September 28, 2022 22:10
@alessbell alessbell force-pushed the issue-10105-loading-stays-true branch from 86bcb6c to 8412af2 Compare September 29, 2022 09:58
@alessbell alessbell changed the base branch from release-3.7 to main October 11, 2022 18:04
@alessbell alessbell force-pushed the issue-10105-loading-stays-true branch from 94a73f1 to 15e8020 Compare October 12, 2022 15:59
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.

I think we can simplify this a bit, and avoid introducing a new public method to the ObservableQuery class, which is a public API. Comments below!

src/core/ObservableQuery.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@alessbell alessbell force-pushed the issue-10105-loading-stays-true branch from c1069de to 94256e6 Compare October 12, 2022 16:10
@alessbell alessbell requested a review from benjamn October 12, 2022 16:15
@alessbell alessbell added this to the 3.7.x patch releases milestone Oct 12, 2022
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.

Looks good to me, thanks @alessbell. Please drop a note in CHANGELOG.md (in a new section for 3.7.1) and merge away!

@alessbell alessbell merged commit ebcec85 into main Oct 17, 2022
@alessbell alessbell deleted the issue-10105-loading-stays-true branch October 24, 2022 19:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 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

Successfully merging this pull request may close these issues.

"Loading" stays true when the same result is returned twice in Apollo Client 3.6.9
4 participants