From dd04acfe15be194f061bd13663258b9e1ac73336 Mon Sep 17 00:00:00 2001 From: Ketan Reddy Date: Tue, 4 Feb 2025 14:41:49 -0500 Subject: [PATCH 1/4] Update FlowManager in Managed Player state if a new one is passed in --- react/player/src/manager/managed-player.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/react/player/src/manager/managed-player.tsx b/react/player/src/manager/managed-player.tsx index 8fa68f7c5..73cce1dff 100644 --- a/react/player/src/manager/managed-player.tsx +++ b/react/player/src/manager/managed-player.tsx @@ -243,7 +243,7 @@ export const usePersistentStateMachine = (options: { return unsub; }, []); - return { managedState, state }; + return { state, managedState }; }; /** @@ -252,7 +252,9 @@ export const usePersistentStateMachine = (options: { * * `suspense` must be enabled to wait for results in flight. */ -export const ManagedPlayer = (props: ManagedPlayerProps) => { +export const ManagedPlayer = ( + props: ManagedPlayerProps, +): React.ReactElement | null => { const { withRequestTime, RequestTimeMetricsPlugin } = useRequestTime(); const { state, managedState } = usePersistentStateMachine({ @@ -284,6 +286,12 @@ export const ManagedPlayer = (props: ManagedPlayerProps) => { }; }, [props.manager, state?.context.reactPlayer.player, state?.value]); + React.useEffect(() => { + if (managedState.state) { + managedState.state.context.manager = props.manager; + } + }, [props.manager]); + if (state?.value === "error") { if (props.fallbackComponent) { return ( From e07d1d3c13c8f8afae27bc1670bd6711eaba7867 Mon Sep 17 00:00:00 2001 From: Ketan Reddy Date: Wed, 5 Feb 2025 09:06:51 -0500 Subject: [PATCH 2/4] Only update flow manager if managed player is in a completed state --- react/player/src/manager/managed-player.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/react/player/src/manager/managed-player.tsx b/react/player/src/manager/managed-player.tsx index 73cce1dff..d0b9779c0 100644 --- a/react/player/src/manager/managed-player.tsx +++ b/react/player/src/manager/managed-player.tsx @@ -287,7 +287,7 @@ export const ManagedPlayer = ( }, [props.manager, state?.context.reactPlayer.player, state?.value]); React.useEffect(() => { - if (managedState.state) { + if (managedState.state?.value === "completed") { managedState.state.context.manager = props.manager; } }, [props.manager]); From ca8c57645e80a00643e9abc42f7f5e6042aba7ae Mon Sep 17 00:00:00 2001 From: Ketan Reddy Date: Wed, 5 Feb 2025 16:05:31 -0500 Subject: [PATCH 3/4] Create new ManagedState when flow manager changes instead of reusing the old one --- react/player/src/manager/managed-player.tsx | 40 +++++++++++++++------ 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/react/player/src/manager/managed-player.tsx b/react/player/src/manager/managed-player.tsx index d0b9779c0..3b123b8ae 100644 --- a/react/player/src/manager/managed-player.tsx +++ b/react/player/src/manager/managed-player.tsx @@ -208,6 +208,18 @@ const managedPlayerStateMachines = new WeakMap< ManagedState >(); +function getOrCreateNewManagedState( + key: ManagedPlayerStateKey, + middleware?: ManagerMiddleware, +): ManagedState { + const newManagedState = + managedPlayerStateMachines.get(key) ?? + new ManagedState({ middleware: middleware }); + managedPlayerStateMachines.set(key, newManagedState); + + return newManagedState; +} + /** Creates an x-state state machine that persists when this component is no longer renders (due to Suspense) */ export const usePersistentStateMachine = (options: { /** the flow manager to use */ @@ -223,12 +235,24 @@ export const usePersistentStateMachine = (options: { _key: Symbol("managed-player"), }); - const managedState = - managedPlayerStateMachines.get(keyRef.current) ?? - new ManagedState({ middleware: options.middleware }); - managedPlayerStateMachines.set(keyRef.current, managedState); + const initialManagedState = getOrCreateNewManagedState( + keyRef.current, + options.middleware, + ); + + const [managedState, setManagedState] = React.useState(initialManagedState); const [state, setState] = React.useState(managedState.state); + React.useEffect(() => { + if (state?.value === "completed") { + const newManagedState = getOrCreateNewManagedState( + keyRef.current, + options.middleware, + ); + setManagedState(newManagedState); + } + }, [options.manager, options.playerConfig]); + React.useEffect(() => { const unsub = managedState.addListener({ onState: (s) => { @@ -241,7 +265,7 @@ export const usePersistentStateMachine = (options: { } return unsub; - }, []); + }, [managedState]); return { state, managedState }; }; @@ -286,12 +310,6 @@ export const ManagedPlayer = ( }; }, [props.manager, state?.context.reactPlayer.player, state?.value]); - React.useEffect(() => { - if (managedState.state?.value === "completed") { - managedState.state.context.manager = props.manager; - } - }, [props.manager]); - if (state?.value === "error") { if (props.fallbackComponent) { return ( From 35636dc2b063ac76101d3ab2417a4d8735c2334c Mon Sep 17 00:00:00 2001 From: Ketan Reddy Date: Thu, 6 Feb 2025 11:04:31 -0500 Subject: [PATCH 4/4] use ref instead of state for managed state. Misc type cleanups --- react/player/src/manager/managed-player.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/react/player/src/manager/managed-player.tsx b/react/player/src/manager/managed-player.tsx index 3b123b8ae..2db9a686c 100644 --- a/react/player/src/manager/managed-player.tsx +++ b/react/player/src/manager/managed-player.tsx @@ -59,7 +59,7 @@ class ManagedState { /** the config to use when creating a player */ playerConfig: ReactPlayerOptions; - }) { + }): this { const initialState: ManagedPlayerState = { value: "not_started", context: { @@ -75,7 +75,7 @@ class ManagedState { } /** reset starts from nothing */ - public reset() { + public reset(): void { if (this.state?.value === "error") { const { playerConfig, manager } = this.state.context; this.start({ playerConfig, manager }); @@ -85,7 +85,7 @@ class ManagedState { } /** restart starts from the last result */ - public restart() { + public restart(): void { if (this.state?.value === "error") { const { playerConfig, manager, prevResult, reactPlayer } = this.state.context; @@ -230,7 +230,7 @@ export const usePersistentStateMachine = (options: { /** Any middleware for the manager */ middleware?: ManagerMiddleware; -}) => { +}): { state: ManagedPlayerState | undefined; managedState: ManagedState } => { const keyRef = React.useRef({ _key: Symbol("managed-player"), }); @@ -240,7 +240,8 @@ export const usePersistentStateMachine = (options: { options.middleware, ); - const [managedState, setManagedState] = React.useState(initialManagedState); + const managedStateRef = React.useRef(initialManagedState); + let managedState = managedStateRef.current; const [state, setState] = React.useState(managedState.state); React.useEffect(() => { @@ -249,7 +250,7 @@ export const usePersistentStateMachine = (options: { keyRef.current, options.middleware, ); - setManagedState(newManagedState); + managedState = newManagedState; } }, [options.manager, options.playerConfig]);