Repository navigation
fix: sync shared-name storage vars in the writing tab (#7567) #7637
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
base: main
Are you sure you want to change the base?
Changes from all commits
344ae42
37476aa
ffde87e
41d7be6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Synced `LocalStorage` vars that share one storage `name` now update in the tab that writes, not only in other tabs. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1172,7 +1172,18 @@ export const connect = async ( | |
| for (const substate in delta) { | ||
| dispatch[substate](delta[substate]); | ||
| } | ||
| applyClientStorageDelta(client_storage, delta); | ||
| const pending_syncs = applyClientStorageDelta(client_storage, delta); | ||
| if (pending_syncs.length > 0) { | ||
| queueEvents( | ||
| pending_syncs.map(({ key, state_keys }) => | ||
| ReflexEvent(SYNC_LOCAL_STORAGE, { key, state_keys }), | ||
| ), | ||
| socket, | ||
| false, | ||
| navigate, | ||
| params, | ||
| ).catch(reportStateUpdateError); | ||
| } | ||
| } | ||
| if (update.events && update.events.length > 0) { | ||
| queueEvents(update.events, socket, false, navigate, params).catch( | ||
|
|
@@ -1363,8 +1374,13 @@ const takeEcho = (state_key, value) => { | |
| * Update client storage values based on backend state delta. | ||
| * @param client_storage The client storage object from context.js | ||
| * @param delta The state update from the backend | ||
| * @returns Storage syncs to send for this tab's sibling vars sharing a | ||
| * written storage name, as [{key, state_keys}]. The storage event reaches | ||
| * other tabs only, so same-tab siblings are synced from the write here. | ||
| */ | ||
| const applyClientStorageDelta = (client_storage, delta) => { | ||
| // Syncs to send, keyed by storage name so one name syncs once per delta. | ||
| const pending_syncs = new Map(); | ||
| // find the main state and check for is_hydrated | ||
| const unqualified_states = Object.keys(delta).filter( | ||
| (key) => key.split(".").length === 1, | ||
|
|
@@ -1377,7 +1393,7 @@ const applyClientStorageDelta = (client_storage, delta) => { | |
| ) { | ||
| // skip if the state is not hydrated yet, since all client storage | ||
| // values are sent in the hydrate event | ||
| return; | ||
| return []; | ||
| } | ||
| } | ||
| // Save known client storage values to cookies and localStorage. | ||
|
|
@@ -1395,7 +1411,8 @@ const applyClientStorageDelta = (client_storage, delta) => { | |
| state_key in client_storage.local_storage && | ||
| typeof window !== "undefined" | ||
| ) { | ||
| const name = client_storage.local_storage[state_key].name || state_key; | ||
| const options = client_storage.local_storage[state_key]; | ||
| const name = options.name || state_key; | ||
| const echo = takeEcho(state_key, value); | ||
| const last = lastLocalStorageWrites[name]; | ||
| // Write an echo only over this tab's own later write, which the backend | ||
|
|
@@ -1406,11 +1423,43 @@ const applyClientStorageDelta = (client_storage, delta) => { | |
| (last?.writes > echo.writes && | ||
| localStorage.getItem(name) === last.value) | ||
| ) { | ||
| const previous = localStorage.getItem(name); | ||
| localStorage.setItem(name, value); | ||
| lastLocalStorageWrites[name] = { | ||
| value, | ||
| writes: ++localStorageWrites, | ||
| }; | ||
| // The storage event fires in other tabs only, so queue the sync for | ||
| // this tab's synced vars sharing the name here, whether or not the | ||
| // written var itself syncs (storage writes raise the event | ||
| // cross-tab either way). Like handleStorage, the sync covers every | ||
| // synced var with the name: when one delta writes several of them, | ||
| // the last write wins in storage, and the sync reads that value. | ||
| // Skip an unchanged value: its vars are already in sync, and | ||
| // re-sending would ping-pong when an echo misses. Deltas are | ||
| // per-session, and a sent update echoes back and takeEcho keeps it | ||
| // from writing again. | ||
| if (previous !== String(value) && !pending_syncs.has(name)) { | ||
| const state_keys = Object.keys(client_storage.local_storage).filter( | ||
| (other_key) => | ||
| client_storage.local_storage[other_key].sync && | ||
| (client_storage.local_storage[other_key].name || other_key) === | ||
| name, | ||
| ); | ||
| // Sync only when another var shares the name. Skip when a synced | ||
| // var with the name has an unechoed sent value: this delta replies | ||
| // to an older event than that value. The echo of the newer value | ||
| // carries every applied key for the name, so storage and the | ||
| // sibling vars converge without a sync from this stale delta. | ||
| if ( | ||
| state_keys.some((other_key) => other_key !== state_key) && | ||
| !state_keys.some( | ||
| (other_key) => sentStorageValues[other_key]?.length > 0, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This guard can permanently suppress sibling sync after a sent value is reconciled to a different backend value. Prompt for AI agents |
||
| ) | ||
|
Comment on lines
+1456
to
+1458
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a Track pending replies separately from the value history, and add a shared-name test with a replaced echo. |
||
| ) { | ||
| pending_syncs.set(name, state_keys); | ||
| } | ||
|
Comment on lines
+1442
to
+1461
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sync can send a value that is older than one this tab already sent for the sibling. The delta is the reply to an earlier event. If the tab sent Sequence with
Both tabs end on Please skip the sibling sync when a synced var with this name has an unechoed entry in Generated by Claude Code
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the guard and the same-tab Playwright regression test. One edge case: if get_delta rewrites or drops a sent value, or its event errors, an unmatched sent entry can suppress later sibling syncs for that name until a matching echo clears it or reconnect/hydrate heals it. While suppressed, the same-tab sibling can remain stale, as on main. I've kept this scoped to your prescribed ordering fix. Is that limitation acceptable here, or should I address it in this PR? |
||
| } | ||
| } | ||
| } else if ( | ||
| client_storage.session_storage && | ||
|
|
@@ -1422,6 +1471,7 @@ const applyClientStorageDelta = (client_storage, delta) => { | |
| } | ||
| } | ||
| } | ||
| return [...pending_syncs].map(([key, state_keys]) => ({ key, state_keys })); | ||
| }; | ||
|
|
||
| /** | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.