Visitar URL original
fix(solid-query): avoid held options reads during cache notifications by niukanen1 · Pull Request #12034 · TanStack/query · GitHub
Skip to content

fix(solid-query): avoid held options reads during cache notifications - #12034

Merged
TkDodo merged 1 commit into
TanStack:solid-query-v6-prefrom
niukanen1:fix/solid-oncacheevent-12031
Nov 1, 2026
Merged

fix(solid-query): avoid held options reads during cache notifications#12034
TkDodo merged 1 commit into
TanStack:solid-query-v6-prefrom
niukanen1:fix/solid-oncacheevent-12031

Conversation

@niukanen1

@niukanen1 niukanen1 commented Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown

🎯 Changes

Closes #12031.

Disposing a query during a transition landing can notify a sibling whose options memo has no committed value. Reading that memo untracked can throw NotReadyError on Solid 2 rc.14+ and interrupt the landing.

Keep the hash last applied by the options render effect in a plain field and match it alongside the latest computed hash. The observer's mutable options cannot serve as that snapshot because fetching and imperative refetching can advance them before the render effect runs (useBaseQuery's data pull and refetch both call observer.setOptions early).

Verified on solid-js 2.0.0-rc.15: the regression test from the issue fails on the previous code with the NotReadyError stack from the issue and passes with this change, in both development and production conditions. Also verified on solid-js/@solidjs/web/@solidjs/signals 2.0.0-rc.19 (devDependency bump, lockfile untouched): the same test fails on the unpatched code with the plain Error the rc.19 dev build re-throws ("Read of an unresolved async source after an await") and passes with this change. On the locked rc.13 runtime the suite is unaffected - the throwing branch does not exist there, and the full package suite shows the same results with and without this change (the pre-existing test-d type-check failures on this machine are identical in both runs).

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm run test:pr.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

@niukanen1
fix(solid-query): avoid held options reads during cache notifications
542eb5c 
Disposing a query during a transition landing can notify a sibling whose
options memo has no committed value. Reading that memo untracked throws
NotReadyError on Solid 2 rc.14+ and interrupts the landing.

Keep the hash last applied by the options render effect in a plain field
and match it alongside the latest computed hash. The observer's mutable
options cannot serve as that snapshot because fetching and imperative
refetching can advance them before the render effect runs.
@changeset-bot

changeset-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 542eb5c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@tanstack/solid-query Patch
@tanstack/solid-query-devtools Patch
@tanstack/solid-query-persist-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a3f0f845-15d6-41da-a58e-68342dbb7ade

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@niukanen1 niukanen1 mentioned this pull request Oct 15, 2026
Open
@niukanen1

niukanen1 commented Oct 22, 2026

Copy link
Copy Markdown
Author

Could a maintainer please approve the pending GitHub Actions workflow runs for this PR when convenient? Thank you.

@neokofg

neokofg commented Oct 28, 2026

Copy link
Copy Markdown

Re-checked against solid-js / @solidjs/web / @solidjs/signals 2.0.0-rc.19, and against next@17d4d1b with signals built from source, using @tanstack/solid-query@6.0.0-rc.5 unpatched. The reproduction from #12031 still fails:

Solid result uncaught error
2.0.0-rc.13 passes (s() === 3) none
2.0.0-rc.15 s() stays 2 NotReadyError
2.0.0-rc.19 / next@17d4d1b s() stays 2, last rendered value 2 Error: Read of an unresolved async source after an \await`. …` (dev build)

On rc.19 the development build no longer surfaces the throw from untrack(defaultedOptions) as NotReadyError. The signals dev-mode check now re-throws it as a plain Error with that message. Code that catches only NotReadyError would therefore miss it in dev.

With a guard in onCacheEvent that treats a throwing untrack(defaultedOptions) as "no committed hash" (the same idea as this PR), the reproduction passes on rc.19 with no uncaught error. Neither the rc.19 release nor next has a change on the Solid side that removes the need for the adapter fix.

@niukanen1

niukanen1 commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Thanks for checking rc.19. I ran this PR's transition-landing test against solid-js/@solidjs/web/@solidjs/signals 2.0.0-rc.19 locally (devDependency bump, lockfile untouched in the PR): it passes with the PR, and on the unpatched code it reproduces exactly what you describe, s() stays 2 and the dev build re-throws the untrack read as "Read of an unresolved async source after an await". The guard in this PR takes the same idea one step earlier: onCacheEvent never reads the options memo at all, it compares against a plain lastAppliedHash field that the render effect updates, so there is no throw to catch in the first place.

@nx-cloud

nx-cloud Bot commented Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 542eb5c

Command Status Duration Result
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 24s View ↗
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 2m 51s View ↗

☁️ Nx Cloud last updated this comment at 2026-10-31 04:17:25 UTC

@pkg-pr-new

pkg-pr-new Bot commented Oct 31, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@12034

@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@12034

@tanstack/lit-query

npm i https://pkg.pr.new/@tanstack/lit-query@12034

@tanstack/preact-query

npm i https://pkg.pr.new/@tanstack/preact-query@12034

@tanstack/preact-query-devtools

npm i https://pkg.pr.new/@tanstack/preact-query-devtools@12034

@tanstack/preact-query-persist-client

npm i https://pkg.pr.new/@tanstack/preact-query-persist-client@12034

@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@12034

@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@12034

@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@12034

@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@12034

@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@12034

@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@12034

@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@12034

@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@12034

@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@12034

@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@12034

@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@12034

@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@12034

@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@12034

@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@12034

@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@12034

@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@12034

@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@12034

@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@12034

commit: 542eb5c

brenelz
brenelz approved these changes Oct 31, 2026
View reviewed changes

@brenelz brenelz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks like a good fix

@neokofg neokofg mentioned this pull request Oct 31, 2026
Open
@TkDodo
TkDodo merged commit 6bab31b into TanStack:solid-query-v6-pre Nov 1, 2026
9 checks passed
@github-actions github-actions Bot mentioned this pull request Nov 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brenelz brenelz brenelz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@niukanen1 @neokofg @brenelz @TkDodo