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

fix(queryObserver): make sure that memoized select function only runs once per input #3098

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Dec 15, 2021

closes #3095

we compute the result optimistically, and then one more time when calling setOptions. Both times run the select function because only setOptions will update the previous result and previous options. storing the "previous" selectFn separately on the observer whenever it was invoked (independent of the fact from where it was called) side-steps the issue

… once per input

we compute the result optimistically, and then one more time when calling setOptions. Both times run the select function because only setOptions will update the previous result and previous options. storing the "previous" selectFn separately on the observer whenever it was invoked (independent of the fact from where it was called) side-steps the issue
@vercel
Copy link

vercel bot commented Dec 15, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tanstack/react-query/9arZ9xZC3Spo2XQVqftMCYgDwByj
✅ Preview: https://react-query-git-fork-tkdodo-feature-3095-double-select-tanstack.vercel.app

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit caef29c:

Sandbox Source
tannerlinsley/react-query: basic Configuration
tannerlinsley/react-query: basic-typescript Configuration
select-runs-twice Issue #3095
select-runs-twice (forked) Issue #3095

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #3098 (caef29c) into master (f9077bc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3098   +/-   ##
=======================================
  Coverage   96.47%   96.47%           
=======================================
  Files          45       45           
  Lines        2269     2270    +1     
  Branches      639      639           
=======================================
+ Hits         2189     2190    +1     
  Misses         77       77           
  Partials        3        3           
Impacted Files Coverage Δ
src/core/queryObserver.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9077bc...caef29c. Read the comment docs.

@TkDodo
Copy link
Collaborator Author

TkDodo commented Dec 15, 2021

@chrislacey89 FYI

@chrislacey89
Copy link

@TkDodo works like a charm. Good work!

@TkDodo TkDodo merged commit 2054f5b into TanStack:master Dec 15, 2021
@TkDodo TkDodo deleted the feature/3095-double-select branch December 15, 2021 18:52
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 3.34.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 4.0.0-alpha.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

TkDodo added a commit to TkDodo/react-query that referenced this pull request Dec 28, 2021
…ted values

not all runs to createResult are saved on `this.currentResult` (optimistically created updates do not - we just create the result again later). With the fix from TanStack#3098, we made sure that memoized select does not run again in those cases by storing every run. However, that now means that we cannot rely on `prevResult.data` because it might still contain outdated data.

The solution is to store a pair of previous selectFn + data and return the exact data that was computed from the previous select fn if we get the exact same function passed
TkDodo added a commit that referenced this pull request Dec 28, 2021
…ted values (#3140)

not all runs to createResult are saved on `this.currentResult` (optimistically created updates do not - we just create the result again later). With the fix from #3098, we made sure that memoized select does not run again in those cases by storing every run. However, that now means that we cannot rely on `prevResult.data` because it might still contain outdated data.

The solution is to store a pair of previous selectFn + data and return the exact data that was computed from the previous select fn if we get the exact same function passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

select runs twice
3 participants