mirror of
https://github.com/vector-im/element-call.git
synced 2026-03-25 06:40:26 +00:00
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. a1110af6d5, 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).
This commit is contained in:
23
src/useLocalStorage.test.tsx
Normal file
23
src/useLocalStorage.test.tsx
Normal file
@@ -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!");
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user