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(react-query): make sure promises are always finalized #8211

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Oct 23, 2024

otherwise, there can be cases with multiple observers where we have listeners already, but still need to finalize

otherwise, there can be cases with multiple observers where we have listeners already, but still need to finalize
Copy link

nx-cloud bot commented Oct 23, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ef9efc2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Copy link

pkg-pr-new bot commented Oct 23, 2024

Open in Stackblitz

More templates

@tanstack/angular-query-devtools-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@8211

@tanstack/eslint-plugin-query

pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@8211

@tanstack/angular-query-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@8211

@tanstack/query-async-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@8211

@tanstack/query-broadcast-client-experimental

pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@8211

@tanstack/query-core

pnpm add https://pkg.pr.new/@tanstack/query-core@8211

@tanstack/query-devtools

pnpm add https://pkg.pr.new/@tanstack/query-devtools@8211

@tanstack/query-sync-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@8211

@tanstack/query-persist-client-core

pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@8211

@tanstack/react-query

pnpm add https://pkg.pr.new/@tanstack/react-query@8211

@tanstack/react-query-devtools

pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@8211

@tanstack/react-query-next-experimental

pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@8211

@tanstack/react-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@8211

@tanstack/solid-query

pnpm add https://pkg.pr.new/@tanstack/solid-query@8211

@tanstack/solid-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@8211

@tanstack/solid-query-devtools

pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@8211

@tanstack/svelte-query

pnpm add https://pkg.pr.new/@tanstack/svelte-query@8211

@tanstack/svelte-query-devtools

pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@8211

@tanstack/svelte-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@8211

@tanstack/vue-query

pnpm add https://pkg.pr.new/@tanstack/vue-query@8211

@tanstack/vue-query-devtools

pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@8211

commit: ef9efc2

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.79%. Comparing base (4fceb26) to head (ef9efc2).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           main    #8211       +/-   ##
=========================================
+ Coverage      0   81.79%   +81.79%     
=========================================
  Files         0       26       +26     
  Lines         0      357      +357     
  Branches      0       97       +97     
=========================================
+ Hits          0      292      +292     
- Misses        0       56       +56     
- Partials      0        9        +9     
Components Coverage Δ
@tanstack/angular-query-devtools-experimental ∅ <ø> (∅)
@tanstack/angular-query-experimental ∅ <ø> (∅)
@tanstack/eslint-plugin-query ∅ <ø> (∅)
@tanstack/query-async-storage-persister ∅ <ø> (∅)
@tanstack/query-broadcast-client-experimental ∅ <ø> (∅)
@tanstack/query-codemods ∅ <ø> (∅)
@tanstack/query-core ∅ <ø> (∅)
@tanstack/query-devtools ∅ <ø> (∅)
@tanstack/query-persist-client-core ∅ <ø> (∅)
@tanstack/query-sync-storage-persister ∅ <ø> (∅)
@tanstack/react-query 93.07% <100.00%> (∅)
@tanstack/react-query-devtools 10.00% <ø> (∅)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (∅)
@tanstack/solid-query ∅ <ø> (∅)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client ∅ <ø> (∅)
@tanstack/svelte-query ∅ <ø> (∅)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client ∅ <ø> (∅)
@tanstack/vue-query ∅ <ø> (∅)
@tanstack/vue-query-devtools ∅ <ø> (∅)

@TkDodo TkDodo merged commit df49fbd into main Oct 23, 2024
8 checks passed
@TkDodo TkDodo deleted the feature/use-multiple-observers branch October 23, 2024 12:51
@@ -150,10 +150,8 @@ export function useBaseQuery<
client.getQueryCache().get(defaultedOptions.queryHash)?.promise

promise?.catch(noop).finally(() => {
if (!observer.hasListeners()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, I should know by now not to do premature optimizations 😬

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😂 seems like we removed all the checks that were initially added because we found a case where that broke it

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.

2 participants