From 144b4dc190ca5f65f35000f724610adaaafd1e9e Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Wed, 10 Jun 2026 23:51:57 +0530 Subject: [PATCH 1/4] fix: Stop mobile editor from reverting keystrokes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit currentText was $state, and the validated-state sync $effect both reads and writes it. Every keystroke (which sets currentText in the CodeMirror updateListener) therefore re-ran the effect while re-validation was still in flight, dispatching a full-document replacement with the stale validatedState text — visibly reverting the keystroke and resetting the cursor until validation caught up. The effect only needs to react to validatedState publishes, so make currentText a plain variable, matching DesktopEditor. No unit test: the repo has no component-test harness, and CodeMirror cannot mount under jsdom without one. Co-Authored-By: Claude Fable 5 --- src/lib/components/MobileEditor.svelte | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/components/MobileEditor.svelte b/src/lib/components/MobileEditor.svelte index 8523c39d..2308b32a 100644 --- a/src/lib/components/MobileEditor.svelte +++ b/src/lib/components/MobileEditor.svelte @@ -15,7 +15,10 @@ let editorView: EditorView | undefined; let editorContainer: HTMLDivElement; - let currentText = $state(''); + // Deliberately not $state: the sync effect below both reads and writes it, + // so a reactive currentText would make every keystroke re-run the effect + // against the not-yet-revalidated state and revert the user's input. + let currentText = ''; const themeCompartment = new Compartment(); const languageCompartment = new Compartment(); From 59105628f4692549341ee262a1342dac78561bf9 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Wed, 10 Jun 2026 23:53:00 +0530 Subject: [PATCH 2/4] fix: Treat a stored JSON null as a missing persisted value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readJSON returned any successfully parsed value, so a localStorage entry holding the literal "null" came back as null instead of the fallback — and a null inputState crashes module init on every load until storage is cleared. The deleted MacFJA persist layer guarded this with `null !== initialValue`; restore that behavior. Co-Authored-By: Claude Fable 5 --- src/lib/util/persist.svelte.test.ts | 12 ++++++++++++ src/lib/util/persist.svelte.ts | 7 ++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/lib/util/persist.svelte.test.ts b/src/lib/util/persist.svelte.test.ts index 3776bb54..fa85c0a5 100644 --- a/src/lib/util/persist.svelte.test.ts +++ b/src/lib/util/persist.svelte.test.ts @@ -22,6 +22,12 @@ describe('readJSON', () => { window.localStorage.setItem('legacy', 'undefined'); expect(readJSON('legacy', 'fallback')).toBe('fallback'); }); + + it('returns the fallback when the stored value parses to null', () => { + // The pre-runes persistence layer treated a stored null as absent. + window.localStorage.setItem('legacy-null', 'null'); + expect(readJSON('legacy-null', 'fallback')).toBe('fallback'); + }); }); describe('writeJSON', () => { @@ -51,4 +57,10 @@ describe('persisted', () => { expect(counter.value).toBe(42); expect(window.localStorage.getItem('counter')).toBe('42'); }); + + it('uses the initial value when storage holds a literal null', () => { + window.localStorage.setItem('settings', 'null'); + const settings = persisted('settings', { theme: 'default' }); + expect(settings.value).toEqual({ theme: 'default' }); + }); }); diff --git a/src/lib/util/persist.svelte.ts b/src/lib/util/persist.svelte.ts index 5204af25..a8c8e0fb 100644 --- a/src/lib/util/persist.svelte.ts +++ b/src/lib/util/persist.svelte.ts @@ -15,7 +15,12 @@ export const readJSON = (key: string, fallback: T): T => { } try { const raw = window.localStorage.getItem(key); - return raw === null ? fallback : (JSON.parse(raw) as T); + if (raw === null) { + return fallback; + } + // A stored literal "null" means the value is absent: the pre-runes + // persistence layer never wrote null and treated it as missing. + return (JSON.parse(raw) as T) ?? fallback; } catch { return fallback; } From d2f067a540f9aeb553670997287680a666117565 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Wed, 10 Jun 2026 23:57:01 +0530 Subject: [PATCH 3/4] refactor: Route every input state mutation through one untracked gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The untrack()/persistAndProcess() pair was a per-function convention: each update function had to remember both, a forgotten untrack would let calling effects subscribe to input state (the bug fixed in 6a9a306e), and a forgotten persistAndProcess would silently skip persistence and re-validation. A single update(mutate) gateway now makes both structural, and the scattered untrack calls (three of which were dead) are gone. Also: - Export inputState as Readonly so writes outside the update functions are type errors instead of comment violations. - Share the 'codeStore' key as a constant instead of two literals. - Reuse validatedStateOf() in processState instead of duplicating the validated-state defaults. - Use $state.raw for validatedCurrent: it is only ever replaced wholesale, so deep proxying every published state was pure overhead. The new state.svelte.test.ts pins the invariants: update functions never make a calling effect track input state, and every mutation persists. Vitest needed resolve.conditions=['browser'] for those tests — it was loading Svelte's server build, where $effect is a no-op. Co-Authored-By: Claude Fable 5 --- src/lib/util/state.svelte.test.ts | 88 ++++++++++++++++++++ src/lib/util/state.svelte.ts | 132 +++++++++++++++++------------- vite.config.js | 2 + 3 files changed, 167 insertions(+), 55 deletions(-) create mode 100644 src/lib/util/state.svelte.test.ts diff --git a/src/lib/util/state.svelte.test.ts b/src/lib/util/state.svelte.test.ts new file mode 100644 index 00000000..4a3ec35a --- /dev/null +++ b/src/lib/util/state.svelte.test.ts @@ -0,0 +1,88 @@ +import type { State } from '$lib/types'; +import { flushSync } from 'svelte'; +import { describe, expect, it } from 'vitest'; +import { + defaultState, + inputState, + loadState, + replaceInputState, + toggleDarkTheme, + updateCode, + updateCodeStore, + updateConfig, + verifyState +} from './state.svelte'; + +// Runs `body` inside an effect and reports how often the effect (re-)runs. +const countEffectRuns = (body: () => void): { runs: () => number; stop: () => void } => { + let runs = 0; + const stop = $effect.root(() => { + $effect(() => { + runs++; + body(); + }); + }); + flushSync(); + return { runs: () => runs, stop }; +}; + +const readStoredState = (): State => + JSON.parse(window.localStorage.getItem('codeStore') ?? '{}') as State; + +describe('update functions called from effects', () => { + // Effects that call an update function must not subscribe to the input + // state the function reads, or unrelated state changes re-fire the effect + // (and self-reads loop, e.g. the dark-theme effect in +layout.svelte). + const cases: [string, () => void][] = [ + ['updateCodeStore', () => updateCodeStore({})], + ['updateCode', () => updateCode('graph TD\n inside-effect')], + ['updateConfig', () => updateConfig('{"theme":"default"}')], + ['toggleDarkTheme', () => toggleDarkTheme(false)], + ['replaceInputState', () => replaceInputState({ ...defaultState })], + ['verifyState', () => verifyState()], + ['loadState', () => loadState('')] + ]; + + it.each(cases)('%s does not make the calling effect track input state', (_name, call) => { + const counter = countEffectRuns(call); + try { + expect(counter.runs()).toBe(1); + updateCode('graph TD\n external-change'); + updateConfig('{"theme":"forest"}'); + updateCodeStore({ pan: { x: 1, y: 2 } }); + flushSync(); + expect(counter.runs()).toBe(1); + } finally { + counter.stop(); + } + }); +}); + +describe('update functions persist input state', () => { + it('updateCode writes the new code to localStorage', () => { + updateCode('graph TD\n persisted-by-test'); + expect(readStoredState().code).toBe('graph TD\n persisted-by-test'); + }); + + it('updateCodeStore merges partial state and persists it', () => { + updateCodeStore({ rough: true }); + expect(inputState.rough).toBe(true); + expect(readStoredState().rough).toBe(true); + }); + + it('replaceInputState drops keys absent from the next state and persists', () => { + updateCodeStore({ pan: { x: 1, y: 2 } }); + expect(inputState.pan).toEqual({ x: 1, y: 2 }); + replaceInputState({ ...defaultState }); + expect(inputState.pan).toBeUndefined(); + expect(readStoredState().pan).toBeUndefined(); + expect(readStoredState().code).toBe(defaultState.code); + }); + + it('verifyState forces panZoom back on', () => { + updateCodeStore({ panZoom: false }); + verifyState(); + expect(inputState.panZoom).toBe(true); + expect(readStoredState().panZoom).toBe(true); + }); +}); diff --git a/src/lib/util/state.svelte.ts b/src/lib/util/state.svelte.ts index 3fe59457..29913377 100644 --- a/src/lib/util/state.svelte.ts +++ b/src/lib/util/state.svelte.ts @@ -42,32 +42,35 @@ const urlParseFailedState = `flowchart TD G --> |"No :("| H(Try using the Timeline tab in History
from same browser you used to create the diagram.) click D href "https://github.com/mermaid-js/mermaid-live-editor/issues/new?assignees=&labels=bug&template=bug_report.md&title=Broken%20link" "Raise issue"`; -// inputState handles all updates and is shared externally when exporting via URL, History, etc. -// It is reactive for reads; every write must go through the update functions -// below so each change is persisted and re-validated. -// The fallback is cloned so mutations never write through to defaultState. -export const inputState = $state(readJSON('codeStore', { ...defaultState })); +const CODE_STORE_KEY = 'codeStore'; -const validatedStateOf = (state: State): ValidatedState => ({ +// The single mutable input state; only update() below may write to it. +// The fallback is cloned so mutations never write through to defaultState. +const input = $state(readJSON(CODE_STORE_KEY, { ...defaultState })); + +// inputState is shared externally when exporting via URL, History, etc. +// It is reactive for reads; the read-only type keeps writes inside this +// module, where update() persists and re-validates every change. +export const inputState: Readonly = input; + +const validatedStateOf = (state: State, serialized: string): ValidatedState => ({ ...state, editorMode: state.editorMode ?? 'code', error: undefined, errorMarkers: [], - serialized: serializeState(state) + serialized }); -let validatedCurrent = $state(validatedStateOf($state.snapshot(inputState))); +const initialState = $state.snapshot(input) as State; +// Only ever replaced wholesale, so raw (shallow) reactivity is enough. +let validatedCurrent = $state.raw( + validatedStateOf(initialState, serializeState(initialState)) +); let lastDiagramType = ''; const processState = async (state: State) => { - const processed: ValidatedState = { - ...state, - editorMode: state.editorMode ?? 'code', - error: undefined, - errorMarkers: [], - serialized: '' - }; + const processed = validatedStateOf(state, ''); // No changes should be done to fields part of `state`. try { processed.serialized = serializeState(state); @@ -127,17 +130,28 @@ let updateHash: ((serialized: string) => void) | undefined; // Persist the current input state and asynchronously re-validate it, // publishing the result to `validatedState` (and the URL hash, once -// initURLSubscription has run). Reads are untracked so effects that call an -// update function don't start depending on the whole input state. +// initURLSubscription has run). Only called from update(), which suppresses +// dependency tracking. const persistAndProcess = (): void => { - const snapshot = untrack(() => $state.snapshot(inputState)); - writeJSON('codeStore', snapshot); + const snapshot = $state.snapshot(input) as State; + writeJSON(CODE_STORE_KEY, snapshot); void processState(snapshot).then((processed) => { validatedCurrent = processed; updateHash?.(processed.serialized); }); }; +// The single mutation gateway: every update function funnels its writes +// through here. The mutator runs untracked so effects that call an update +// function never subscribe to the input state it reads, and the trailing +// persist + re-validate cannot be forgotten by a new update function. +const update = (mutate: (state: State) => void): void => { + untrack(() => { + mutate(input); + persistAndProcess(); + }); +}; + // All internal reads should be done via validatedState, but it should not be // persisted/shared externally. export const validatedState = { @@ -274,27 +288,32 @@ export const sanitizeConfig = (config: string | MermaidConfig) => { }; export const loadState = (data: string): void => { - let state: State; console.log(`Loading '${data}'`); - try { - state = deserializeState(data); - state.mermaid = sanitizeConfig(state.mermaid || defaultState.mermaid); - } catch (error) { - state = untrack(() => $state.snapshot(inputState)); - if (data) { - console.error('Init error', error); - state.code = urlParseFailedState; - state.mermaid = defaultState.mermaid; + update((state) => { + let next: State; + try { + next = deserializeState(data); + next.mermaid = sanitizeConfig(next.mermaid || defaultState.mermaid); + } catch (error) { + next = $state.snapshot(state) as State; + if (data) { + console.error('Init error', error); + next.code = urlParseFailedState; + next.mermaid = defaultState.mermaid; + } } - } - updateCodeStore(state); + applyPartial(state, next); + }); }; let renderCount = 0; -export const updateCodeStore = (newState: Partial): void => { +const applyPartial = (state: State, newState: Partial): void => { renderCount++; - Object.assign(inputState, newState, { renderCount }); - persistAndProcess(); + Object.assign(state, newState, { renderCount }); +}; + +export const updateCodeStore = (newState: Partial): void => { + update((state) => applyPartial(state, newState)); }; export const updateCode = ( @@ -306,13 +325,14 @@ export const updateCode = ( ): void => { errorDebug(); - if (resetPanZoom) { - inputState.pan = undefined; - inputState.zoom = undefined; - } - inputState.code = code; - inputState.updateDiagram = updateDiagram; - persistAndProcess(); + update((state) => { + if (resetPanZoom) { + state.pan = undefined; + state.zoom = undefined; + } + state.code = code; + state.updateDiagram = updateDiagram; + }); }; export const updateConfig = (config: string): void => { @@ -320,25 +340,27 @@ export const updateConfig = (config: string): void => { }; export const toggleDarkTheme = (dark: boolean): void => { - const config = JSON.parse(untrack(() => inputState.mermaid)) as MermaidConfig; - if (!config.theme || ['dark', 'default'].includes(config.theme)) { - config.theme = dark ? 'dark' : 'default'; - } - inputState.mermaid = formatJSON(config); - persistAndProcess(); + update((state) => { + const config = JSON.parse(state.mermaid) as MermaidConfig; + if (!config.theme || ['dark', 'default'].includes(config.theme)) { + config.theme = dark ? 'dark' : 'default'; + } + state.mermaid = formatJSON(config); + }); }; // Replaces the whole input state (e.g. when restoring a history entry), // dropping keys the next state does not define. export const replaceInputState = (next: State): void => { - for (const key of untrack(() => Object.keys(inputState))) { - if (!(key in next)) { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- full-replace semantics - delete (inputState as unknown as Record)[key]; + update((state) => { + for (const key of Object.keys(state)) { + if (!(key in next)) { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- full-replace semantics + delete (state as unknown as Record)[key]; + } } - } - Object.assign(inputState, next); - persistAndProcess(); + Object.assign(state, next); + }); }; export const initURLSubscription = (): void => { @@ -349,5 +371,5 @@ export const initURLSubscription = (): void => { }; export const verifyState = (): void => { - updateCodeStore(untrack(() => inputState.panZoom) ? {} : { panZoom: true }); + update((state) => applyPartial(state, state.panZoom ? {} : { panZoom: true })); }; diff --git a/vite.config.js b/vite.config.js index 48f853eb..d04a88a2 100644 --- a/vite.config.js +++ b/vite.config.js @@ -33,6 +33,8 @@ export default defineConfig({ envPrefix: 'MERMAID_', server: { port: 3000, host: true }, preview: { port: 3000, host: true }, + // Vitest otherwise resolves Svelte's server build, where $effect is a no-op. + resolve: process.env.VITEST ? { conditions: ['browser'] } : undefined, test: { environment: 'jsdom', // in-source testing From c2530549165a5185b19d355d505fe2dd27e66af6 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Wed, 10 Jun 2026 23:57:27 +0530 Subject: [PATCH 4/4] refactor: Make persisted() values raw state The getter handed out a deep $state proxy, so in-place mutation of a persisted array/record would update the UI while silently never being written to localStorage. All consumers already replace .value wholesale; $state.raw makes that the only semantics and drops the per-read proxy overhead. Co-Authored-By: Claude Fable 5 --- src/lib/util/persist.svelte.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/util/persist.svelte.ts b/src/lib/util/persist.svelte.ts index a8c8e0fb..34b092a2 100644 --- a/src/lib/util/persist.svelte.ts +++ b/src/lib/util/persist.svelte.ts @@ -37,8 +37,10 @@ export interface Persisted { } // A localStorage-backed reactive value. Reads on init, writes on every set. +// Raw state: replace `value` wholesale to change it. With a deep proxy, +// in-place mutation would update the UI without ever being persisted. export const persisted = (key: string, initial: T): Persisted => { - let value = $state(readJSON(key, initial)); + let value = $state.raw(readJSON(key, initial)); return { get value() { return value;