mirror of
https://github.com/vector-im/element-call.git
synced 2026-01-30 03:15:55 +00:00
b0587fcfb3
* Avoid reactivity bugs in how we track external state
Many of our hooks which attempt to bridge external state from an EventEmitter or EventTarget into React had subtle bugs which could cause them to fail to react to certain updates. The conditions necessary for triggering these bugs are explained by the tests that I've included.
In the majority of cases, I don't think we were triggering these bugs in practice. They could've become problems if we refactored our components in certain ways. The one concrete case I'm aware of in which we actually triggered such a bug was the race condition with the useRoomEncryptionSystem shared secret logic (addressed by a1110af6d5).
But, particularly with all the weird reactivity issues we're debugging this week, I think we need to eliminate the possibility that any of the bugs in these hooks are the cause of our current headaches.
* Reuse useTypedEventEmitterState in useLocalStorage
* Fix type error
87 lines
2.3 KiB
TypeScript
87 lines
2.3 KiB
TypeScript
/*
|
|
Copyright 2022-2024 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 { useCallback, useEffect, useSyncExternalStore } from "react";
|
|
|
|
import type {
|
|
Listener,
|
|
ListenerMap,
|
|
TypedEventEmitter,
|
|
} from "matrix-js-sdk/lib/models/typed-event-emitter";
|
|
|
|
/**
|
|
* Shortcut for registering a listener on an EventTarget.
|
|
*/
|
|
export function useEventTarget<T extends Event>(
|
|
target: EventTarget | null | undefined,
|
|
eventType: string,
|
|
listener: (event: T) => void,
|
|
options?: AddEventListenerOptions,
|
|
): void {
|
|
useEffect(() => {
|
|
if (target) {
|
|
target.addEventListener(eventType, listener as EventListener, options);
|
|
return (): void =>
|
|
target.removeEventListener(
|
|
eventType,
|
|
listener as EventListener,
|
|
options,
|
|
);
|
|
}
|
|
}, [target, eventType, listener, options]);
|
|
}
|
|
|
|
/**
|
|
* Shortcut for registering a listener on a TypedEventEmitter.
|
|
*/
|
|
export function useTypedEventEmitter<
|
|
Events extends string,
|
|
Arguments extends ListenerMap<Events>,
|
|
T extends Events,
|
|
>(
|
|
emitter: TypedEventEmitter<Events, Arguments>,
|
|
eventType: T,
|
|
listener: Listener<Events, Arguments, T>,
|
|
): void {
|
|
useEffect(() => {
|
|
emitter.on(eventType, listener);
|
|
return (): void => {
|
|
emitter.off(eventType, listener);
|
|
};
|
|
}, [emitter, eventType, listener]);
|
|
}
|
|
|
|
/**
|
|
* Reactively tracks a value which is recalculated whenever the provided event
|
|
* emitter emits an event. This is useful for bridging state from matrix-js-sdk
|
|
* into React.
|
|
*/
|
|
export function useTypedEventEmitterState<
|
|
Events extends string,
|
|
Arguments extends ListenerMap<Events>,
|
|
T extends Events,
|
|
State,
|
|
>(
|
|
emitter: TypedEventEmitter<Events, Arguments>,
|
|
eventType: T,
|
|
getState: () => State,
|
|
): State {
|
|
const subscribe = useCallback(
|
|
(onChange: () => void) => {
|
|
emitter.on(eventType, onChange as Listener<Events, Arguments, T>);
|
|
return (): void => {
|
|
emitter.off(eventType, onChange as Listener<Events, Arguments, T>);
|
|
};
|
|
},
|
|
[emitter, eventType],
|
|
);
|
|
// See the React docs for useSyncExternalStore; given that we're trying to
|
|
// bridge state from an external source into React, using this hook is exactly
|
|
// what React recommends.
|
|
return useSyncExternalStore(subscribe, getState);
|
|
}
|