perf: cache shadow-DOM styles, debounce draft persistence#8
Conversation
Two low-risk, no-API-change quick wins identified while profiling the
widget.
1. `createStyles()` now memoizes its output at module load. The function
returns a ~50 KB CSS string that is identical for every render across
the lifetime of the page. We were re-allocating that string on every
theme apply / shadow re-render. Subsequent calls now return the
cached reference.
2. `persistDraftRound()` now coalesces rapid calls into a single
`localStorage` write via `queueMicrotask`. The 20+ call sites
throughout the widget (input handlers, attachment progress, drag,
markup pointer events, etc.) frequently fire multiple persists in
the same task — they all collapse to one write at end-of-task.
A new `flushPersistDraftRound()` provides synchronous write
guarantees for critical paths:
- `destroy()` — flush before tearing down so we never lose state.
- Submit-success paths (both standard and visual-suggestion) —
immediately commit the cleared draft.
- `beforeunload` — defensive flush during normal page navigation.
The class also short-circuits scheduled persists after `destroy()`
to avoid writing stale state from a torn-down widget.
No public API changes. No behavior changes for consumers. Bundle delta
is ~+1.4 KB minified (debounce infrastructure).
There was a problem hiding this comment.
Obvious Code Review
Readiness:
Findings
- Blocker: 0
- High: 0
- Medium: 1
- Suggestion: 1
- Nit: 1
Medium — queueMicrotask coalesces within a task, not across events
src/index.ts — new persistDraftRound() method (~line 4058)
The JSDoc claims savings on "burst paths (typing, dragging, scrolling)" but queueMicrotask fires at the end of the current microtask checkpoint — before the next browser task. Each keydown/pointermove/scroll arrives as a separate task, so the microtask from event N has already fired before event N+1 begins. The actual win is narrower: it only merges multiple synchronous calls within a single code path (e.g. several methods calling persistDraftRound() before returning). For true per-keystroke debouncing, replace queueMicrotask with setTimeout(fn, 100–200). The flushPersistDraftRound() mechanism already handles the "must write before unload/submit" guarantee correctly regardless of which timer is used.
Suggestion — back-to-back schedule + flush is indirect at submit-success sites
Lines ~4046, ~6488, ~6734: persistDraftRound() followed immediately by flushPersistDraftRound() works correctly (flag prevents double-write) but the intent is "write now unconditionally." Consider a persistDraftRoundNow() alias or making flushPersistDraftRound() write unconditionally regardless of the flag.
Nit — cachedStyles lacks safety comment
Line 1036: the cache is safe because the interpolated constants (DEFAULT_TRIGGER_SIZE_PX etc.) are compile-time. A brief comment prevents a future reader from adding a runtime-configurable value and inadvertently caching stale CSS.
| */ | ||
| private persistDraftRound(): void { | ||
| if (this.persistDraftScheduled || this.destroyed) { | ||
| return; |
There was a problem hiding this comment.
Medium — queueMicrotask fires before the next browser task, not after a time window. Each keydown/pointermove event is its own task, so this does not coalesce across them. The JSDoc and PR title overstate the guarantee. For true burst-path debouncing, use setTimeout(fn, 100–200) instead — flushPersistDraftRound() already covers the synchronous flush requirement.
Summary
Two low-risk, zero-API-change quick wins identified while profiling the widget. Branched from
mainso it can land independently of #7.1. Memoize `createStyles()`
The shadow-DOM CSS is a ~50 KB string literal built fresh on every call. There are 7+ `shadowRoot.innerHTML` overwrites that each call it. Cache the result at module load — the string is identical for the lifetime of the page.
2. Coalesce `persistDraftRound()` writes via `queueMicrotask`
There are 20+ call sites for `persistDraftRound()` (input handlers, attachment progress, drag, markup pointer events, etc.). Many fire multiple writes in the same task. `queueMicrotask` collapses them into a single `localStorage.setItem` at end-of-task.
A new `flushPersistDraftRound()` is called from paths that need a synchronous write:
Scheduled persists are short-circuited after `destroy()` to avoid writing stale state from a torn-down widget.
Bundle delta
The ~1.4 KB cost buys the debounce infrastructure + flush hooks. Worth it for the write-coalescing on burst paths.
Test plan
Why not more?
This PR is intentionally tiny so it can land first as a baseline. Larger structural wins (event delegation to avoid per-render rebinding, file split, code splitting) are planned as follow-up branches.