From 5409c0ac6ebe033abf608b8ebaf9c440f5b5d410 Mon Sep 17 00:00:00 2001 From: Carlos Valente <34649812+cpvalente@users.noreply.github.com> Date: Sat, 30 Dec 2023 21:13:51 +0100 Subject: [PATCH] refactor: stabilise actionHandler (#683) --- apps/client/src/common/api/apiConstants.ts | 1 + apps/client/src/common/hooks/useMemoisedFn.ts | 38 +++++ .../src/features/rundown/RundownEntry.tsx | 160 ++++++++---------- 3 files changed, 111 insertions(+), 88 deletions(-) create mode 100644 apps/client/src/common/hooks/useMemoisedFn.ts diff --git a/apps/client/src/common/api/apiConstants.ts b/apps/client/src/common/api/apiConstants.ts index 4ad3a12f3..e0364c012 100644 --- a/apps/client/src/common/api/apiConstants.ts +++ b/apps/client/src/common/api/apiConstants.ts @@ -13,6 +13,7 @@ export const RUNTIME = ['runtimeStore']; const location = window.location; const socketProtocol = location.protocol === 'https:' ? 'wss' : 'ws'; export const isProduction = import.meta.env.MODE === 'production'; +export const isDev = !isProduction; const STATIC_PORT = 4001; export const serverPort = isProduction ? location.port : STATIC_PORT; diff --git a/apps/client/src/common/hooks/useMemoisedFn.ts b/apps/client/src/common/hooks/useMemoisedFn.ts new file mode 100644 index 000000000..d5507e97c --- /dev/null +++ b/apps/client/src/common/hooks/useMemoisedFn.ts @@ -0,0 +1,38 @@ +/** + * Shamelessly from https://ahooks.js.org/hooks/use-memoized-fn/ + * Interesting technique discussed by Dan Abramov + * https://overreacted.io/making-setinterval-declarative-with-react-hooks/ + */ + +import { useMemo, useRef } from 'react'; + +import { isDev } from '../api/apiConstants'; + +type noop = (this: any, ...args: any[]) => any; + +type PickFunction = (this: ThisParameterType, ...args: Parameters) => ReturnType; + +export const isFunction = (value: unknown): value is (...args: any) => any => typeof value === 'function'; + +export default function useMemoisedFn(fn: T) { + if (isDev) { + if (!isFunction(fn)) { + console.error(`useMemoisedFn expected function as parameter, got ${typeof fn}`); + } + } + + const fnRef = useRef(fn); + + // why not write `fnRef.current = fn`? + // https://github.com/alibaba/hooks/issues/728 + fnRef.current = useMemo(() => fn, [fn]); + + const memoizedFn = useRef>(); + if (!memoizedFn.current) { + memoizedFn.current = function (this, ...args) { + return fnRef.current.apply(this, args); + }; + } + + return memoizedFn.current as T; +} diff --git a/apps/client/src/features/rundown/RundownEntry.tsx b/apps/client/src/features/rundown/RundownEntry.tsx index 41d917404..a29364b26 100644 --- a/apps/client/src/features/rundown/RundownEntry.tsx +++ b/apps/client/src/features/rundown/RundownEntry.tsx @@ -4,6 +4,7 @@ import { calculateDuration, getCueCandidate } from 'ontime-utils'; import { RUNDOWN } from '../../common/api/apiConstants'; import { useEventAction } from '../../common/hooks/useEventAction'; +import useMemoisedFn from '../../common/hooks/useMemoisedFn'; import { ontimeQueryClient } from '../../common/queryClient'; import { useAppMode } from '../../common/stores/appModeStore'; import { useEditorSettings } from '../../common/stores/editorSettings'; @@ -61,95 +62,78 @@ export default function RundownEntry(props: RundownEntryProps) { value: unknown; }; - // we assume the data is not changing in the lifecycle of this component - // changes to the data would make rundown re-render, also re-rendering this component - const actionHandler = useCallback( - (action: EventItemActions, payload?: number | FieldValue) => { - switch (action) { - case 'event': { - const newEvent = { type: SupportedEvent.Event }; - const options = { - startTimeIsLastEnd, - defaultPublic, - lastEventId: previousEventId, - after: data.id, - }; - addEvent(newEvent, options); - break; - } - case 'delay': { - addEvent({ type: SupportedEvent.Delay }, { after: data.id }); - break; - } - case 'block': { - addEvent({ type: SupportedEvent.Block }, { after: data.id }); - break; - } - case 'swap': { - const { value } = payload as FieldValue; - swapEvents({ from: value as string, to: data.id }); - - break; - } - case 'delete': { - if (openId === data.id) { - removeOpenEvent(); - } - deleteEvent(data.id); - break; - } - case 'clone': { - const newEvent = cloneEvent(data as OntimeEvent, data.id); - const rundown = ontimeQueryClient.getQueryData(RUNDOWN)?.rundown ?? [] - newEvent.cue = getCueCandidate(rundown, data.id); - addEvent(newEvent); - break; - } - case 'update': { - // Handles and filters update requests - const { field, value } = payload as FieldValue; - const newData: Partial = { id: data.id }; - - if (field === 'durationOverride' && data.type === SupportedEvent.Event) { - // duration defines timeEnd - newData.duration = value as number; - newData.timeEnd = data.timeStart + (value as number); - updateEvent(newData); - } else if (field === 'timeStart' && data.type === SupportedEvent.Event) { - newData.duration = calculateDuration(value as number, data.timeEnd); - newData.timeStart = value as number; - updateEvent(newData); - } else if (field === 'timeEnd' && data.type === SupportedEvent.Event) { - newData.duration = calculateDuration(data.timeStart, value as number); - newData.timeEnd = value as number; - updateEvent(newData); - } else if (field in data) { - // @ts-expect-error not sure how to type this - newData[field] = value; - updateEvent(newData); - } else { - emitError(`Unknown field: ${field}`); - } - break; - } - default: - throw new Error(`Unhandled event ${action}`); + const actionHandler = useMemoisedFn((action: EventItemActions, payload?: number | FieldValue) => { + switch (action) { + case 'event': { + const newEvent = { type: SupportedEvent.Event }; + const options = { + startTimeIsLastEnd, + defaultPublic, + lastEventId: previousEventId, + after: data.id, + }; + addEvent(newEvent, options); + break; } - }, - [ - addEvent, - data, - defaultPublic, - deleteEvent, - emitError, - openId, - previousEventId, - removeOpenEvent, - startTimeIsLastEnd, - updateEvent, - swapEvents, - ], - ); + case 'delay': { + addEvent({ type: SupportedEvent.Delay }, { after: data.id }); + break; + } + case 'block': { + addEvent({ type: SupportedEvent.Block }, { after: data.id }); + break; + } + case 'swap': { + const { value } = payload as FieldValue; + swapEvents({ from: value as string, to: data.id }); + + break; + } + case 'delete': { + if (openId === data.id) { + removeOpenEvent(); + } + deleteEvent(data.id); + break; + } + case 'clone': { + const newEvent = cloneEvent(data as OntimeEvent, data.id); + const rundown = ontimeQueryClient.getQueryData(RUNDOWN)?.rundown ?? []; + newEvent.cue = getCueCandidate(rundown, data.id); + addEvent(newEvent); + break; + } + case 'update': { + // Handles and filters update requests + const { field, value } = payload as FieldValue; + const newData: Partial = { id: data.id }; + + if (field === 'durationOverride' && data.type === SupportedEvent.Event) { + // duration defines timeEnd + newData.duration = value as number; + newData.timeEnd = data.timeStart + (value as number); + updateEvent(newData); + } else if (field === 'timeStart' && data.type === SupportedEvent.Event) { + newData.duration = calculateDuration(value as number, data.timeEnd); + newData.timeStart = value as number; + updateEvent(newData); + } else if (field === 'timeEnd' && data.type === SupportedEvent.Event) { + newData.duration = calculateDuration(data.timeStart, value as number); + newData.timeEnd = value as number; + updateEvent(newData); + } else if (field in data) { + // @ts-expect-error not sure how to type this + newData[field] = value; + updateEvent(newData); + } else { + emitError(`Unknown field: ${field}`); + } + break; + } + default: + throw new Error(`Unhandled event ${action}`); + } + }); if (data.type === SupportedEvent.Event) { return (