Repository navigation
fix(signals): an optimistic guess's replaced row reaches <For> - #4360
Merged
Merged
Conversation
The container's arrangement comparator judged rows by key, keyless rows by
position. A guess that put a different object at an index with the same
key (a fresh `{ id }` row returned from the setter), or any keyless row
(identical rows swapped in the draft, a fresh row with the same contents,
a reorder: #4344), was "the same row": the arrangement guess was dropped
and `<For>` kept the old row's proxy while direct reads served the new
one, so an edit of that row never reached the screen.
A row the store already serves is now its proxy's row: another object at
the index is another row. A landing's object is adopted by the row it
pairs with before the comparator runs, so an echo still confirms (#3898).
Rows no proxy serves yet still pair by key, keyless rows by position.
The comparator also folds its keyless special case into `sameKey`
(`undefined` is the same key), so the store scenario gets smaller.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2
🦋 Changeset detectedLatest commit: 3f79fb2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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 40677694999Coverage remained the same at 78.727%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will degrade performance by 4.59%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
Member
Author
|
CodSpeed flagged Generated by Claude Code |
ryansolid
pushed a commit
that referenced
this pull request
Oct 28, 2026
Brings in #4347, #4350-#4353, #4356, #4357, #4359 and #4360. The one conflict was in the store-app entry of scripts/size/scenarios.js. Both ledgers are kept: next's two lines, then this branch's two. The cap is re-measured after the merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2
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
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.
Fixes #4344. This supersedes #4358: same issue, a broader rule, fewer bytes. The two PRs edit the same lines of
rowSame, so only one of them should land.Summary
Before: a guess on a
createOptimisticStorecan put a different row object at an index that the container's arrangement comparator still calls "the same row". When that happens, the arrangement guess is dropped.<For>(keyed and non-keyed) keeps the old row's proxy, while direct reads serve the new row, so an edit of that row shows in direct reads but not in the DOM. Each case below does that swap or return, thend[0].n = 2inside an action:next2,1, screen1,2idreturned from the setter2,1, screen1,12,1, screen1,1c,b,a, screena,b,cPlain
createStoreandcreateProjectionagree with direct reads in every case on rc.19 andnext. Only the optimistic guess path has this bug.Cause:
rowSame(the comparator behindarrangement→arrangementEqual) matched keyed rows by key and keyless rows by position. A landing's object is adopted into the row it pairs with, so its proxy survives. A guess's fresh object instead wraps a new proxy, and a draft swap moves two different proxies. In both cases "same key" or "same position" no longer means "same proxy", but the comparator still said "same". SooptimisticWritedropped the arrangement guess and<For>never re-ran. The revert at the settle had the same problem.After: a row the store already serves is its proxy's row.
lookupTarget(raw, fam)is checked on both sides. If either side has a target, the rows are the same row only when the targets are equal. A landing's object has already been adopted by the time the comparator runs, so a server echo still confirms (#3898 passes unchanged). Rows that no proxy serves yet still pair by key, and keyless rows by position. The keyless special case now goes throughsameKey, sinceundefinedis the same key. That also makes the comparator smaller.I did not reuse reconcile's adopt path (
reconcileState/pairRows) for the setter's returned array. That would keep proxies only for keyed matches. It would not cover A (no new objects, just two different proxies moved), and keyless rows would need a new positional fallback. The comparator change covers every case without new code paths.How did you test this change?
packages/signals/tests/optimistic-replaced-row-identity.test.ts:mapArray, exact frames at the guess and after the settle.createStoreandcreateProjectioncontrols.packages/signals/tests/optimistic-reorder-keyless-4344.test.tsandpackages/web/test/optimistic-reorder-keyless-for-4344.spec.tsx.packages/web/test/optimistic-replaced-row-for.spec.tsx: cases A, B and C under<For>keyed andkeyed={false}, exactinnerHTMLat the guess and after the settle.next, all optimistic cases fail (the 8 web ones included) and every control passes. With this change, all pass.tsc -p tsconfig.build.jsonis clean.nextin this environment (uncommitted-input-overwritten,server-functions-adapter-request"keeps a rebuild failure's own error…",lone-spread-hydration-key-4240with a locally stale native compiler).scripts/size, measured againstnext@ 38e0c49 on the same machine: onlyapp: hydrating + every store primitive familychanges, by −27 B minified (95,595 → 95,568) and −33 B brotli. No cap changes; it is now under its cap.Generated by Claude Code