From 6258bcec549c1b743601dff5e2c4869f97de8e61 Mon Sep 17 00:00:00 2001
From: Robin <robin@robin.town>
Date: Wed, 28 May 2025 12:18:18 -0400
Subject: [PATCH] Fix a possible race in useLocalStorage

This race could cause useLocalStorage to entirely fail to react to an update to the item in question.

It seems we are hitting this race condition in practice with the useRoomEncryptionSystem shared secret logic. a1110af6d549e2e3b5cd33912e0a6389d7bdf326, I think, causes us to depend on the existence of this race. But really we ought to fix the race and also fix useRoomEncryptionSystem to not depend on it (PR from Timo forthcoming).
---
 src/useLocalStorage.test.tsx | 23 +++++++++++++++++++++++
 src/useLocalStorage.ts       | 13 ++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 src/useLocalStorage.test.tsx

diff --git a/src/useLocalStorage.test.tsx b/src/useLocalStorage.test.tsx
new file mode 100644
index 00000000..4b0a058d
--- /dev/null
+++ b/src/useLocalStorage.test.tsx
@@ -0,0 +1,23 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE in the repository root for full details.
+*/
+
+import { test } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { type FC, useEffect } from "react";
+
+import { setLocalStorageItem, useLocalStorage } from "./useLocalStorage";
+
+test("useLocalStorage reacts to changes made by an effect mounted on the same render", () => {
+  localStorage.clear();
+  const Test: FC = () => {
+    useEffect(() => setLocalStorageItem("my-value", "Hello!"), []);
+    const [myValue] = useLocalStorage("my-value");
+    return myValue;
+  };
+  render(<Test />);
+  screen.getByText("Hello!");
+});
diff --git a/src/useLocalStorage.ts b/src/useLocalStorage.ts
index 1394e0d3..517c7c62 100644
--- a/src/useLocalStorage.ts
+++ b/src/useLocalStorage.ts
@@ -8,6 +8,8 @@ Please see LICENSE in the repository root for full details.
 import EventEmitter from "events";
 import { useCallback, useEffect, useState } from "react";
 
+import { useLatest } from "./useLatest";
+
 type LocalStorageItem = ReturnType<typeof localStorage.getItem>;
 
 // Bus to notify other useLocalStorage consumers when an item is changed
@@ -20,13 +22,22 @@ export const useLocalStorage = (
   const [value, setValue] = useState<LocalStorageItem>(() =>
     localStorage.getItem(key),
   );
+  const latestValue = useLatest(value);
 
   useEffect(() => {
+    // We're about to set up the bus listener that will enable us to react to
+    // any future updates to the localStorage item. However, it's possible that
+    // we already missed an update if there was an effect which modified the
+    // item in the time *between* the render phase of useLocalStorage and the
+    // execution of this effect. Let's update the state if that happened.
+    const stored = localStorage.getItem(key);
+    if (latestValue.current !== stored) setValue(stored);
+
     localStorageBus.on(key, setValue);
     return (): void => {
       localStorageBus.off(key, setValue);
     };
-  }, [key, setValue]);
+  }, [key, latestValue, setValue]);
 
   return [
     value,