diff --git a/packages/reflex-base/news/+storage-sync-shared-name.bugfix.md b/packages/reflex-base/news/+storage-sync-shared-name.bugfix.md new file mode 100644 index 00000000000..5b32a1d94ed --- /dev/null +++ b/packages/reflex-base/news/+storage-sync-shared-name.bugfix.md @@ -0,0 +1 @@ +Editing one tab no longer leaves other `sync=True` browser storage vars that share its storage `name` stale in other tabs; every such var is now synced, not only the last one declared. diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js index 2336a33340a..c4f212de66c 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/state.js @@ -458,13 +458,15 @@ function urlFrom(string) { * * The value is read when the event is sent, so it and the write count recorded * with it come from the same moment. - * @param payload The storage name and state key of the var. + * @param payload The storage name and the state keys of the vars that use it. * @returns The update_vars_internal event. */ -const syncLocalStorageEvent = ({ key, state_key }) => - ReflexEvent(`${app.state_name}.${UPDATE_VARS_INTERNAL}`, { - vars: { [state_key]: localStorage.getItem(key) }, +const syncLocalStorageEvent = ({ key, state_keys }) => { + const value = localStorage.getItem(key); + return ReflexEvent(`${app.state_name}.${UPDATE_VARS_INTERNAL}`, { + vars: Object.fromEntries(state_keys.map((state_key) => [state_key, value])), }); +}; /** * Handle frontend event or send the event to the backend via Websocket. @@ -1522,26 +1524,33 @@ export const useEventLoop = ( // localStorage event handling useEffect(() => { - const storage_to_state_map = {}; + // A Map, so a storage name like "constructor" is not an inherited property. + const storage_to_state_map = new Map(); if (client_storage.local_storage && typeof window !== "undefined") { for (const state_key in client_storage.local_storage) { const options = client_storage.local_storage[state_key]; if (options.sync) { const local_storage_value_key = options.name || state_key; - storage_to_state_map[local_storage_value_key] = state_key; + // Several vars can share one storage name; sync all of them. + const state_keys = storage_to_state_map.get(local_storage_value_key); + if (state_keys) { + state_keys.push(state_key); + } else { + storage_to_state_map.set(local_storage_value_key, [state_key]); + } } } } // e is StorageEvent const handleStorage = (e) => { - const state_key = storage_to_state_map[e.key]; + const state_keys = storage_to_state_map.get(e.key); // Session storage changes in same-origin frames raise storage events too. - if (state_key && e.storageArea === localStorage) { + if (state_keys && e.storageArea === localStorage) { // The value is read when the event is sent: see syncLocalStorageEvent. addEvents( - [ReflexEvent(SYNC_LOCAL_STORAGE, { key: e.key, state_key })], + [ReflexEvent(SYNC_LOCAL_STORAGE, { key: e.key, state_keys })], e, ); } diff --git a/tests/units/compiler/state_js.test.mjs b/tests/units/compiler/state_js.test.mjs index df8472228fb..1583138edb2 100644 --- a/tests/units/compiler/state_js.test.mjs +++ b/tests/units/compiler/state_js.test.mjs @@ -18,6 +18,7 @@ async function setup({ disabled = false, hidden = false, storedToken = "original-token", + clientStorage = {}, react = { useCallback() {}, useEffect() {}, useRef() {}, useState() {} }, } = {}) { const sockets = []; @@ -32,6 +33,12 @@ async function setup({ getItem: (key) => storage.get(key), setItem: (key, value) => storage.set(key, value), }; + const localStorage = { + items: new Map(), + getItem(key) { + return this.items.get(key) ?? null; + }, + }; const document = new EventTarget(); document.cookie = disabled ? "backend-enabled=false" : ""; document.visibilityState = hidden ? "hidden" : "visible"; @@ -41,6 +48,7 @@ async function setup({ URL, URLSearchParams, performance, + localStorage, ...(browser ? { window, document } : {}), queueMicrotask: (fn) => microtasks.push(fn), setTimeout: (fn, ms) => { @@ -127,6 +135,7 @@ async function setup({ initialState: stateful ? { root: {}, child: {} } : {}, onLoadInternalEvent() {}, state_name: "root", + clientStorage, exception_state_name: "exception", }, eventLoop: { addEvents() {}, connectErrors: [] }, @@ -164,6 +173,7 @@ async function setup({ timers, firstHydrates, window, + localStorage, socket, updates, module, @@ -314,7 +324,7 @@ for (const config of [ } /** Run `useEventLoop` once with minimal hooks, keeping the effects it registers. */ -async function mountEventLoop() { +async function mountEventLoop(clientStorage = {}) { const effects = []; const sent = []; const emitted = { @@ -323,6 +333,7 @@ async function mountEventLoop() { }; let refs = 0; const harness = await setup({ + clientStorage, react: { useCallback: (fn) => fn, useEffect: (fn) => effects.push(fn), @@ -334,7 +345,7 @@ async function mountEventLoop() { harness.module.namespace.useEventLoop( () => {}, () => [], - {}, + clientStorage, ); return { harness, effects, sent }; } @@ -393,3 +404,66 @@ test("window.onerror preserves the original Error message", async () => { ); assert.equal(event.payload.component_stack, ""); }); + +test("a storage event syncs every synced var that shares the storage name", async () => { + const clientStorage = { + local_storage: { + first: { name: "shared", sync: true }, + second: { name: "shared", sync: true }, + other: { name: "other", sync: true }, + unsynced: { name: "shared" }, + }, + }; + const { harness, effects, sent } = await mountEventLoop(clientStorage); + for (const effect of effects) effect(); + harness.localStorage.items.set("shared", "stored"); + + const event = new Event("storage"); + event.key = "shared"; + event.storageArea = harness.localStorage; + harness.window.dispatchEvent(event); + await new Promise((resolve) => setImmediate(resolve)); + + const updates = sent.filter((e) => e.name.endsWith("update_vars_internal")); + assert.equal(updates.length, 1, JSON.stringify(sent)); + // The vars come from the vm's realm, so compare their serialised form. + assert.equal( + JSON.stringify(updates[0].payload.vars), + JSON.stringify({ first: "stored", second: "stored" }), + ); +}); + +test("storage names that are Object.prototype properties sync like any other", async () => { + const clientStorage = { + local_storage: { + first: { name: "constructor", sync: true }, + second: { name: "constructor", sync: true }, + third: { name: "__proto__", sync: true }, + }, + }; + const { harness, effects, sent } = await mountEventLoop(clientStorage); + assert.doesNotThrow(() => { + for (const effect of effects) effect(); + }); + harness.localStorage.items.set("constructor", "stored"); + + for (const key of ["constructor", "toString", "__proto__"]) { + const event = new Event("storage"); + event.key = key; + event.storageArea = harness.localStorage; + harness.window.dispatchEvent(event); + } + await new Promise((resolve) => setImmediate(resolve)); + + // Only the names a var declared are synced, and a name's vars all are. + const updates = sent.filter((e) => e.name.endsWith("update_vars_internal")); + assert.equal(updates.length, 2, JSON.stringify(sent)); + assert.equal( + JSON.stringify(updates[0].payload.vars), + JSON.stringify({ first: "stored", second: "stored" }), + ); + assert.equal( + JSON.stringify(updates[1].payload.vars), + JSON.stringify({ third: null }), + ); +});