From a010cc983f588b602cf67baecd267a71d5bc571e Mon Sep 17 00:00:00 2001 From: "Timo K." Date: Tue, 8 Sep 2026 13:49:19 +0200 Subject: [PATCH] Compare the component's config by value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything downstream of the component's params — the mute state, the call view model and with it the media connection — is keyed on the identity of the params object, which was memoised on the identity of the `config` prop. A host writing `config={{ ... }}` inline, which is the natural way to write it, therefore tore the whole call down on every render. The harness happened to pass a constant, so nothing noticed. `useStableValue` hands out the same object for as long as a deep comparison says nothing changed, so an inline config costs nothing. Co-Authored-By: Claude Fable 5.1 --- component/index.tsx | 14 +++++++-- src/useStableValue.test.ts | 64 ++++++++++++++++++++++++++++++++++++++ src/useStableValue.ts | 29 +++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 src/useStableValue.test.ts create mode 100644 src/useStableValue.ts diff --git a/component/index.tsx b/component/index.tsx index 84a18353c..b58782be0 100644 --- a/component/index.tsx +++ b/component/index.tsx @@ -66,6 +66,7 @@ import { type ConfigOptions } from "../src/config/ConfigOptions"; import { i18n } from "../src/utils/i18n"; import { useTheme } from "../src/useTheme"; import { useInitial } from "../src/useInitial"; +import { useStableValue } from "../src/useStableValue"; import styles from "./ElementCall.module.css"; // Everything needed to implement a HostBridge, not just the interface itself @@ -115,6 +116,9 @@ export interface ElementCallProps { * How Element Call should behave, overriding whatever {@link intent} implies. * A host that finds itself setting a lot of these probably wants a different * intent instead. + * + * Compared by value, so it is fine to write this inline; only a change to + * what it says restarts anything. */ config?: ElementCallConfiguration; /** @@ -180,14 +184,20 @@ export const ElementCall: FC = ({ // Element Call has no URL of its own to read any of this from, and the // host's URL is not Element Call's business, so the defaults come from the // intent with the host's wishes over the top. + // + // Everything downstream — the mute state, the call view model and with it + // the media connection — is keyed on the identity of this object, so it has + // to be stable for as long as its contents are. A host writing `config` + // inline would otherwise tear the call down on every render. + const stableConfig = useStableValue(config); const params = useMemo( (): UrlParams => ({ ...hostedProperties, roomId, ...configurationForIntent(intent), - ...config, + ...stableConfig, }), - [roomId, intent, config], + [roomId, intent, stableConfig], ); const mediaDevices = useInitial( diff --git a/src/useStableValue.test.ts b/src/useStableValue.test.ts new file mode 100644 index 000000000..8759cbafd --- /dev/null +++ b/src/useStableValue.test.ts @@ -0,0 +1,64 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { renderHook } from "@testing-library/react"; +import { describe, expect, test } from "vitest"; + +import { useStableValue } from "./useStableValue"; + +describe("useStableValue", () => { + test("keeps the first identity while the contents stay equal", () => { + const first = { skipLobby: true, fonts: ["Inter"] }; + const { result, rerender } = renderHook( + ({ value }) => useStableValue(value), + { initialProps: { value: first } }, + ); + expect(result.current).toBe(first); + + rerender({ value: { skipLobby: true, fonts: ["Inter"] } }); + expect(result.current).toBe(first); + }); + + test("takes the new identity once the contents change", () => { + const first = { skipLobby: true }; + const second = { skipLobby: false }; + const { result, rerender } = renderHook( + ({ value }) => useStableValue(value), + { initialProps: { value: first } }, + ); + + rerender({ value: second }); + expect(result.current).toBe(second); + + // And that identity is then the stable one + rerender({ value: { skipLobby: false } }); + expect(result.current).toBe(second); + }); + + test("handles undefined, for an optional prop left out", () => { + const { result, rerender } = renderHook( + ({ value }) => useStableValue(value), + { initialProps: { value: undefined as { a: number } | undefined } }, + ); + expect(result.current).toBeUndefined(); + + const given = { a: 1 }; + rerender({ value: given }); + expect(result.current).toBe(given); + }); + + test("accepts its own notion of equality", () => { + const first = { id: 1, label: "a" }; + const { result, rerender } = renderHook( + ({ value }) => useStableValue(value, (a, b) => a.id === b.id), + { initialProps: { value: first } }, + ); + + rerender({ value: { id: 1, label: "b" } }); + expect(result.current).toBe(first); + }); +}); diff --git a/src/useStableValue.ts b/src/useStableValue.ts new file mode 100644 index 000000000..890832166 --- /dev/null +++ b/src/useStableValue.ts @@ -0,0 +1,29 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { useState } from "react"; +import { isEqual } from "lodash-es"; + +/** + * Returns a value whose identity only changes when its contents do. + * + * For a prop that a caller is likely to write inline — an options object, say + * — so that a fresh but equal object on every render does not restart whatever + * depends on it. Deep equality by default. + */ +export function useStableValue( + value: T, + equals: (a: T, b: T) => boolean = isEqual, +): T { + const [stable, setStable] = useState(value); + if (equals(stable, value)) return stable; + // Setting state during render makes React re-run this render immediately + // with the new state, at which point the two are identical and the stored + // one is returned — so the identity handed out is consistent. + setStable(value); + return value; +}