Repository navigation
fix(signals): awaited refresh returns the source, not the caller override (#3895) - #3931
Merged
Merged
Conversation
…ride (#3895) A refresh waiter reads its target through laneRead. A written guess stands in for that node's own in-flight re-ask, so awaiting refresh() of the optimistic accessor resolved with the caller's override while the source flight was still pending. The screen corrected when the source landed; the promise had already settled. A fresh-pull of a guessed node now parks until that flight lands or rejects, then serves the truth staged beneath the guess. until() has no fresh-pull bit, so it still sees the base the guess covers. The waiter's root and the action transaction it was created in stay, which is what delivers a staged landing when refresh targets the underlying source. Co-authored-by: Grok via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: c25c268 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
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 |
Size (brotli, eager entry chunk)
|
Coverage Report for CI Build 37859863127Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.1%) to 76.593%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Merging this PR will improve performance by 10.48%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | projection derive: write one NESTED field (reference) |
2.4 ms | 2.1 ms | +14.15% |
| ⚡ | memo + sync render effect only (reference) |
32.5 ms | 30.4 ms | +6.93% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/3895-refresh-optimistic-authority (c25c268) with next (0c4bff7)
laneRead ships in every isPending/latest bundle. Only refresh() needs to ignore a guess while that node's flight is up, so the park lives in the waiter and pages that never call refresh stop paying for it. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Awaiting
refresh()of an optimistic accessor inside the action that wrote the override returned the caller's guess. The refresh waiter reads the target throughlaneRead, and a written guess stands in for that node's own in-flight re-ask, so the promise settled with the overlay while the source flight was still pending.A fresh-pull of a guessed node now parks until that flight lands or rejects, then serves the truth staged beneath the guess.
until()is unchanged (it has no fresh-pull bit). The waiter's root and the action transaction it is created in stay, so a refresh of the underlying source still delivers the staged landing.Fixes #3895
Public API
None
How did you test this change?
From the worktree, after
pnpm install:pnpm --filter @solidjs/signals exec vitest run tests/refresh-optimistic-authority-3895.test.tsfailed on current next before the fix (expected 99 to be 2).pnpm --filter @solidjs/signals exec vitest run tests/refresh-optimistic-authority-3895.test.ts tests/refresh-await.test.ts tests/until.test.ts tests/createOptimistic.test.ts— 4 files, 101 tests, all passed.pnpm --filter @solidjs/signals test— 5056 passed. The 27 failures are alltests/dist-artifacts.test.ts(ENOENT/Cannot find modulefordist/), because this worktree has no package build. Signals unit tests import source.refresh()'screateRootwaiter from [2.0 next, regressed after rc.13] Production refresh remains pending after a settled computation (production build) #3888 is untouched.The new tests cover the issue repro (source answers 2, override is 99), a re-ask that lands a new value (5, not the pre-refresh commit and not the override), a re-ask that confirms the override, and a rejected re-ask.
Made with Cursor