fix: sync every storage var that shares a name across tabs (#7545)

Co-authored-by: Farhan Ali Raza <62690310+FarhanAliRaza@users.noreply.github.com>

Author: flcrom

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.

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,
         );
       }

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 }),
+  );
+});