setRundown deep-cloned every rundown a second time on top of the clone
createTransaction already makes - the only two callers that matter for
correctness are the ones passing the live cachedRundown singleton, and
db.data.rundowns[key] is only ever replaced wholesale, never mutated in
place, so aliasing it introduces no corruption path. The other six call
sites already pass a freshly-built object they never touch again, for
which the clone was pure waste.
setRundown now takes ownership of `newData` and stores it by reference,
documented on the function. Verified with a throwaway suite (deleted,
per instructions, once green) covering:
- the reference stored is the exact one passed in, not a copy
- round-trip content correctness is unchanged
- a mutation to the source after setRundown is visible through
getRundown, which is the new contract, not a regression - pinned
explicitly so it reads as intentional
- the fresh-object call sites (duplicate, import, rename, etc.) are
unaffected
- mergeRundown / deleteRundown / not-found still behave correctly
- perf: setRundown on a 1000-entry rundown dropped from ~557us/call to
~5us/call
Real disk I/O could not be exercised in that suite - lowdb's JSONFilePreset
forces an in-memory adapter whenever NODE_ENV=test, which vitest always
sets, independent of this project's own IS_TEST flag. That's fine: this
change only affects which object reference ends up at db.data.rundowns[key],
not persist()'s debounce/write-scheduling logic, which is untouched, so
checking that state directly is equivalent to checking what a real write
would serialize.
Full suite (705 tests), typecheck, and lint all pass on the final diff,
which is 8 lines changed in one file.
The per-type clone functions from the previous commit listed every field of
every entry type, which was a lot of code for a change meant to be a drop-in
swap. They are replaced by the spread they were written to guard, keeping both
review fixes and the exhaustiveness guard.
The explicit literals were also weaker than they looked. They forced a new
field to be named, but nothing stopped it being named as a plain alias, which
is the actual bug we care about. The guarantee now comes from a test that
walks a clone against its source and fails on any shared nested object or
array. That covers a field added later without the clone enumerating fields,
and it names the path it found:
entry.triggers.0 is shared with the source
Paired with a field-for-field comparison against structuredClone, the two tests
pin both halves of the contract - same value, no shared references - and both
were confirmed to fail when the corresponding fix is reverted.
Dropping the literals also removes the `parent: undefined` divergence they
introduced on entries that omit the key, since a spread copies exactly the keys
that are present.
Net: the diff for this PR goes from 261 to 159 added lines, with
rundown.utils.ts down from 132 to 52.
Replaces the generic type machinery around cloneEntryData with a concrete
clone per entry type, and fixes two review findings.
Types: ReferenceKeys / EntryOfType / ClonedReferenceFields /
UnclonedReferenceFields / AssertNever are gone. There are only four entry
types, so each gets its own small function listing every field explicitly -
the same pattern the create*Patch functions in this file already use. Adding
a field to an entry type still fails to compile until it is handled, but now
with a plain "property is missing" error rather than a constraint on never,
and the clone reads as the logic it is instead of type gymnastics. The
exhaustiveness guard in the switch is kept, so a new SupportedEntry member is
still named in a compile error.
Fixes, both from review:
- triggers were copied with slice(), which shares every Trigger object with
the source. A mutable transaction could edit a trigger of the cached
rundown before commit. Each trigger is now copied too, making the
structuredClone contract actually true - custom and entries were already
deep since their values are primitives.
- triggers and entries were normalised from undefined to [], so an entry
missing either field never compared equal to its own snapshot. That would
have the runtime re-broadcast and re-save the restore point on every tick.
An absent value is now left absent.
Tests: cloneEntryData is compared field-for-field against structuredClone for
all four entry types built from the real factories, plus aliasing coverage for
custom, triggers (array and elements), and group entries, the absent-value
regression, and the unknown-type throw. Each was confirmed to fail when the
corresponding fix is reverted.
The remaining review comment, about the missing default branch, was already
fixed in 4548798.
cloneEntryData relies on a spread plus a hand-written list of the nested
containers to copy. Nothing tied that list to the real types, so adding a
reference-typed field to an entry - or a new entry type - would silently
produce a clone that aliases the new field back to the source, which is the
exact bug the custom clone exists to avoid.
Two compile-time guards, both verified by temporarily mutating the shared
types:
- ClonedReferenceFields declares, per entry type, the reference-typed fields
the switch copies. UnclonedReferenceFields diffs that against the fields
the types actually have, computed via ReferenceKeys. Adding
`attachments: string[]` to OntimeEvent now fails with
`Type '"attachments"' does not satisfy the constraint 'never'`, naming the
offending field. Adding a primitive field stays silent, since the spread
already copies it by value and no action is needed.
- A default branch in the switch asserts the entry is never. Adding a
SupportedEntry member fails with `Type 'OntimeMarker' is not assignable to
type 'never'`, and separately at the ClonedReferenceFields index, which is
no longer total.
Branded primitives such as Day (number & Brand<'day'>) correctly classify as
primitives, so they are not flagged.
Typecheck, lint, format and the full suite (695 tests) pass unchanged.
Overload createTransaction() on the literal mutableRundown option: with
mutableRundown: true it returns rundown: Rundown as before; otherwise it
returns rundown: DeepReadonly<Rundown> (ts-essentials, already a convention
in this codebase for read-only snapshots).
Previously a non-mutable transaction's rundown was typed as plain Rundown
even though it's the live cachedRundown reference itself (or a background
rundown read straight from disk) - nothing stopped a future mutation
function from writing into it outside the commit() flow, since
Readonly<T> (used elsewhere for the same purpose) only blocks top-level
reassignment, not nested writes like array.push() or entry.field = x.
Scoped to rundown.dao.ts only: every existing call site in rundown.service.ts
passes a literal mutableRundown: true, so this changes no call-site types.
The one mutableRundown: false site doesn't destructure rundown at all.
Verified with a throwaway probe file (removed) that mutating a non-mutable
transaction's rundown is now a compile error, and that a mutable one still
compiles as before. Typecheck, lint and full test suite (695 tests) pass
unchanged.
structuredClone's generic serialization algorithm does far more work than
plain object spreads need for these known shapes. Adds cloneEntryData()
and cloneRundown() as drop-in replacements (same "independent copy" contract,
same call sites) and swaps them in everywhere a rundown or a single entry
was being deep-cloned via structuredClone:
- createTransaction()/init() in rundown.dao.ts - the main per-mutation clone
- DataProvider.setRundown() - was re-cloning the whole rundown a second time
on every single commit
- rundown.service.ts background-rundown clones (custom field rename/remove,
duplicateExistingRundown)
- the per-entry clone in processRundown's non-mutating path (rundown.parser.ts)
- mergeRundownPreservingFields's per-entry clone
Also:
- safeMerge() (DataProvider.utils.ts) was deep-cloning the entire DatabaseModel,
including all rundowns, just to read a handful of small config properties
that never touch rundowns - it now only clones the properties it actually
merges.
- sheets.service.ts's per-row clone before building a (read-only) Google
Sheets cell request was unnecessary and is removed.
- runtime.service.ts's previous-state snapshot for eventNow/eventNext/
eventFlag/groupNow now uses cloneEntryData.
Benchmarked on a synthetic 1000-event rundown: a cue-only edit (no
reprocessing needed) went from ~5ms to ~1.2ms end to end, including the
DataProvider clone. Verified against the existing test suite (695 passing)
plus typecheck and lint.
* fix: issue where a count-to-end would lead to incorrect expected times
* fix: include add time in overtime when countToEnd
* fix: ui and server use same calculation for expected end
The server serves html/timer-legacy.html and html/login.html from disk
at runtime, relative to the bundled server. The Docker image copies
these files but the electron packaging did not, so the view 404ed in
all desktop distributions.
Additionally, AppImages mount at /tmp/.mount_*, a hidden directory.
Express sendFile refuses paths containing dot-segments by default
(returns 404 without touching disk), so the view failed on Linux even
with the file packaged. Allow dotfiles for this route; the request
path is fixed so no user input is affected.
The Stage Timer and PiP Timer views relied on `align-content: center` to
vertically center the timer digits inside `.timer-container`, which is a
plain block element. Aligning children of a block container via
`align-content` only works in Chromium 123+ (Firefox 125+, Safari 17.4+).
Embedded browsers such as vMix Browser Input (CEF V115) and other older
CEF/Chromium-based production tools ignore the property, so the digits
fall back to the top of the container while every other element renders
correctly.
Make `.timer-container` a real flex column and center with
`justify-content: center`, which is universally supported and matches the
centering approach used elsewhere in the codebase. The removed
`justify-self`/`align-self` were no-ops on a full-width flex item.
Fixes#2126
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014T6ENZ3r6JXZb1fpYw2oNY
Claude
Carlos Valente
Carlos Valente
Alex Christoffer Rasmussen
will@t1d.dev