mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-30 19:39:10 +00:00
perf(rundown): keep entry revisions in step with the server
An entry revision advances on every change to that entry, which makes it a cheap marker for what has moved on in a rundown. The client did not maintain it: an optimistic entry carried the old revision, so the entry the server returned always differed from ours by that one field. That difference is enough to defeat structural sharing. A new entries object recomputes the rundown metadata, which fires the effect behind the list, which re-renders every visible row. Editing one title on a fourteen entry rundown cost eight structural and sixty-one attribute mutations, twice over. - optimistic patches apply the same revision bump the server does, so an unchanged entry keeps its identity and a refetch costs nothing to render - the rundown metadata memo keys on the fields the derivation reads, so a change of rundown revision alone no longer churns the list - editing an entry now reads the server response, which is the authority on what was applied, and only writes when it differs from what we predicted Measured on the same edit: eight to four structural, sixty-one to forty-one attribute mutations. Both changes are needed, neither helps on its own. Two bugs found on the way. A swap stored the pre increment revision, so swapped entries were indistinguishable from untouched ones. The optimistic batch edit spread the request envelope onto the entry rather than the patch, so it never applied the edit it was predicting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011a5cbVjNC5XXF88b2PkUCa
This commit is contained in:
@@ -23,7 +23,11 @@ export function useLoadedRundown() {
|
||||
export function useLoadedRundownWithMetadata() {
|
||||
const { data, status } = useLoadedRundown();
|
||||
const selectedEventId = useSelectedEventId();
|
||||
const rundownMetadata = useMemo(() => getRundownMetadata(data, selectedEventId), [data, selectedEventId]);
|
||||
const { entries, flatOrder } = data;
|
||||
const rundownMetadata = useMemo(
|
||||
() => getRundownMetadata({ entries, flatOrder }, selectedEventId),
|
||||
[entries, flatOrder, selectedEventId],
|
||||
);
|
||||
return { data, status, rundownMetadata };
|
||||
}
|
||||
|
||||
@@ -36,7 +40,11 @@ export function useLoadedFlatRundown() {
|
||||
export function useLoadedFlatRundownWithMetadata() {
|
||||
const { data, status } = useLoadedRundown();
|
||||
const selectedEventId = useSelectedEventId();
|
||||
const rundownWithMetadata = useMemo(() => getFlatRundownMetadata(data, selectedEventId), [data, selectedEventId]);
|
||||
const { entries, flatOrder } = data;
|
||||
const rundownWithMetadata = useMemo(
|
||||
() => getFlatRundownMetadata({ entries, flatOrder }, selectedEventId),
|
||||
[entries, flatOrder, selectedEventId],
|
||||
);
|
||||
return { data: rundownWithMetadata, status };
|
||||
}
|
||||
|
||||
|
||||
@@ -27,7 +27,12 @@ export function useScopedSelectedEventId(): EntryId | null {
|
||||
export function useRundownWithMetadata() {
|
||||
const { data, status } = useRundown();
|
||||
const selectedEventId = useScopedSelectedEventId();
|
||||
const rundownMetadata = useMemo(() => getRundownMetadata(data, selectedEventId), [data, selectedEventId]);
|
||||
// key on the fields the derivation reads, a revision only change must not churn the list
|
||||
const { entries, flatOrder } = data;
|
||||
const rundownMetadata = useMemo(
|
||||
() => getRundownMetadata({ entries, flatOrder }, selectedEventId),
|
||||
[entries, flatOrder, selectedEventId],
|
||||
);
|
||||
return { data, status, rundownMetadata };
|
||||
}
|
||||
|
||||
@@ -46,7 +51,11 @@ export function useFlatRundownWithMetadata() {
|
||||
const { data, status } = useRundown();
|
||||
const selectedEventId = useScopedSelectedEventId();
|
||||
|
||||
const rundownWithMetadata = useMemo(() => getFlatRundownMetadata(data, selectedEventId), [data, selectedEventId]);
|
||||
const { entries, flatOrder } = data;
|
||||
const rundownWithMetadata = useMemo(
|
||||
() => getFlatRundownMetadata({ entries, flatOrder }, selectedEventId),
|
||||
[entries, flatOrder, selectedEventId],
|
||||
);
|
||||
return { data: rundownWithMetadata, status };
|
||||
}
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
swapEventData,
|
||||
} from 'ontime-utils';
|
||||
import { useCallback, useMemo } from 'react';
|
||||
import isEqual from 'react-fast-compare';
|
||||
|
||||
import { moveDown, moveUp, orderEntries } from '../../features/rundown/rundown.utils';
|
||||
import { getRundownCacheKey } from '../api/constants';
|
||||
@@ -64,6 +65,22 @@ export type EventOptions = Partial<{
|
||||
lastEventId: MaybeString;
|
||||
}>;
|
||||
|
||||
/**
|
||||
* Applies a patch the way the server does, revision included.
|
||||
*
|
||||
* An entry revision advances on every change to that entry, which makes it a
|
||||
* cheap marker for whether an entry has moved on. The optimistic entry has to
|
||||
* carry the revision the server will return, otherwise the two disagree and the
|
||||
* refetch resolves to a different object for no reason.
|
||||
* Mirrors applyPatchToEntry in the rundown service: delays carry no revision.
|
||||
*/
|
||||
function patchEntry(entry: OntimeEntry, patch: Partial<OntimeEntry>): OntimeEntry {
|
||||
if (isOntimeEvent(entry) || isOntimeGroup(entry) || isOntimeMilestone(entry)) {
|
||||
return { ...entry, ...patch, revision: entry.revision + 1 } as OntimeEntry;
|
||||
}
|
||||
return { ...entry, ...patch } as OntimeEntry;
|
||||
}
|
||||
|
||||
type ClientInsertOptions = {
|
||||
after?: EntryId;
|
||||
before?: EntryId;
|
||||
@@ -321,8 +338,10 @@ function useEntryActionsForRundown(scopedRundownId: string) {
|
||||
if (previousData && eventId) {
|
||||
// optimistically update object
|
||||
const newRundown = { ...previousData.entries };
|
||||
// @ts-expect-error -- we expect the events to be of same type
|
||||
newRundown[eventId] = { ...newRundown[eventId], ...newEvent };
|
||||
const previousEntry = newRundown[eventId];
|
||||
if (previousEntry) {
|
||||
newRundown[eventId] = patchEntry(previousEntry, newEvent);
|
||||
}
|
||||
queryClient.setQueryData<Rundown>(queryKey, {
|
||||
id: previousData.id,
|
||||
title: previousData.title,
|
||||
@@ -336,6 +355,23 @@ function useEntryActionsForRundown(scopedRundownId: string) {
|
||||
// Return a context with the previous and new events
|
||||
return { previousData, newEvent, queryKey };
|
||||
},
|
||||
// the server is the authority on the applied patch, it may normalise what we sent
|
||||
onSuccess: (response, _variables, context) => {
|
||||
const serverEntry = response.data;
|
||||
if (!serverEntry || !context?.queryKey) return;
|
||||
|
||||
const cachedRundown = queryClient.getQueryData<Rundown>(context.queryKey);
|
||||
if (!cachedRundown) return;
|
||||
|
||||
// our optimistic entry usually describes the change exactly, writing an
|
||||
// identical entry would discard the cached reference for nothing
|
||||
if (isEqual(cachedRundown.entries[serverEntry.id], serverEntry)) return;
|
||||
|
||||
queryClient.setQueryData<Rundown>(context.queryKey, {
|
||||
...cachedRundown,
|
||||
entries: { ...cachedRundown.entries, [serverEntry.id]: serverEntry },
|
||||
});
|
||||
},
|
||||
// Mutation fails, rollback undoes optimist update
|
||||
onError: (_error, _newEvent, context) => {
|
||||
if (context?.previousData && context?.queryKey) {
|
||||
@@ -508,10 +544,7 @@ function useEntryActionsForRundown(scopedRundownId: string) {
|
||||
if (Object.hasOwn(newRundown, eventId)) {
|
||||
const event = newRundown[eventId];
|
||||
if (isOntimeEvent(event)) {
|
||||
newRundown[eventId] = {
|
||||
...event,
|
||||
...data,
|
||||
};
|
||||
newRundown[eventId] = patchEntry(event, data.data);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1718,16 +1718,18 @@ describe('rundownMutation.swap()', () => {
|
||||
expect((testRundown.entries['1'] as OntimeEvent).id).toBe('1');
|
||||
expect((testRundown.entries['1'] as OntimeEvent).cue).toBe('data2');
|
||||
expect((testRundown.entries['1'] as OntimeEvent).timeStart).toBe(1);
|
||||
expect((testRundown.entries['1'] as OntimeEvent).revision).toBe(1);
|
||||
// the swapped entries changed, their revision has to say so
|
||||
expect((testRundown.entries['1'] as OntimeEvent).revision).toBe(2);
|
||||
|
||||
expect((testRundown.entries['2'] as OntimeEvent).id).toBe('2');
|
||||
expect((testRundown.entries['2'] as OntimeEvent).cue).toBe('data1');
|
||||
expect((testRundown.entries['2'] as OntimeEvent).timeStart).toBe(2);
|
||||
expect((testRundown.entries['2'] as OntimeEvent).revision).toBe(1);
|
||||
expect((testRundown.entries['2'] as OntimeEvent).revision).toBe(2);
|
||||
|
||||
expect((testRundown.entries['3'] as OntimeEvent).id).toBe('3');
|
||||
expect((testRundown.entries['3'] as OntimeEvent).cue).toBe('data3');
|
||||
expect((testRundown.entries['3'] as OntimeEvent).timeStart).toBe(3);
|
||||
// untouched, so it keeps its revision
|
||||
expect((testRundown.entries['3'] as OntimeEvent).revision).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -422,7 +422,7 @@ function swap(rundown: Rundown, eventFrom: OntimeEvent, eventTo: OntimeEvent) {
|
||||
gap: eventFrom.gap,
|
||||
dayOffset: eventFrom.dayOffset,
|
||||
// keep revision number but increment it
|
||||
revision: eventFrom.revision++,
|
||||
revision: eventFrom.revision + 1,
|
||||
};
|
||||
|
||||
rundown.entries[eventTo.id] = {
|
||||
@@ -440,7 +440,7 @@ function swap(rundown: Rundown, eventFrom: OntimeEvent, eventTo: OntimeEvent) {
|
||||
gap: eventTo.gap,
|
||||
dayOffset: eventTo.dayOffset,
|
||||
// keep revision number but increment it
|
||||
revision: eventTo.revision++,
|
||||
revision: eventTo.revision + 1,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,10 @@ Load only for touched domains. Add only stable, recurring invariants; not one-of
|
||||
- Preserve entry identity and supported types across patch, clone, group, ungroup, reorder.
|
||||
- Distinguish loaded vs background rundown. Prefer explicit rundown ID over global current state.
|
||||
- No caller-owned rundown mutation unless explicitly contracted.
|
||||
- Revisions mark change: a rundown revision advances on every commit, an entry revision on every
|
||||
change to that entry. Delays carry no revision. Optimistic client updates apply the same bump the
|
||||
server does, so an unchanged entry keeps its identity and clients can compare revisions to tell
|
||||
what moved on.
|
||||
|
||||
## Persistence, realtime, cache
|
||||
|
||||
|
||||
Reference in New Issue
Block a user