refactor: Route every input state mutation through one untracked gateway
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<State> 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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
59105628f4
commit
d2f067a540
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -42,32 +42,35 @@ const urlParseFailedState = `flowchart TD
|
|||||||
G --> |"No :("| H(Try using the Timeline tab in History <br/>from same browser you used to create the diagram.)
|
G --> |"No :("| H(Try using the Timeline tab in History <br/>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"`;
|
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.
|
const CODE_STORE_KEY = 'codeStore';
|
||||||
// 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<State>(readJSON('codeStore', { ...defaultState }));
|
|
||||||
|
|
||||||
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<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<State> = input;
|
||||||
|
|
||||||
|
const validatedStateOf = (state: State, serialized: string): ValidatedState => ({
|
||||||
...state,
|
...state,
|
||||||
editorMode: state.editorMode ?? 'code',
|
editorMode: state.editorMode ?? 'code',
|
||||||
error: undefined,
|
error: undefined,
|
||||||
errorMarkers: [],
|
errorMarkers: [],
|
||||||
serialized: serializeState(state)
|
serialized
|
||||||
});
|
});
|
||||||
|
|
||||||
let validatedCurrent = $state<ValidatedState>(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<ValidatedState>(
|
||||||
|
validatedStateOf(initialState, serializeState(initialState))
|
||||||
|
);
|
||||||
|
|
||||||
let lastDiagramType = '';
|
let lastDiagramType = '';
|
||||||
|
|
||||||
const processState = async (state: State) => {
|
const processState = async (state: State) => {
|
||||||
const processed: ValidatedState = {
|
const processed = validatedStateOf(state, '');
|
||||||
...state,
|
|
||||||
editorMode: state.editorMode ?? 'code',
|
|
||||||
error: undefined,
|
|
||||||
errorMarkers: [],
|
|
||||||
serialized: ''
|
|
||||||
};
|
|
||||||
// No changes should be done to fields part of `state`.
|
// No changes should be done to fields part of `state`.
|
||||||
try {
|
try {
|
||||||
processed.serialized = serializeState(state);
|
processed.serialized = serializeState(state);
|
||||||
@@ -127,17 +130,28 @@ let updateHash: ((serialized: string) => void) | undefined;
|
|||||||
|
|
||||||
// Persist the current input state and asynchronously re-validate it,
|
// Persist the current input state and asynchronously re-validate it,
|
||||||
// publishing the result to `validatedState` (and the URL hash, once
|
// publishing the result to `validatedState` (and the URL hash, once
|
||||||
// initURLSubscription has run). Reads are untracked so effects that call an
|
// initURLSubscription has run). Only called from update(), which suppresses
|
||||||
// update function don't start depending on the whole input state.
|
// dependency tracking.
|
||||||
const persistAndProcess = (): void => {
|
const persistAndProcess = (): void => {
|
||||||
const snapshot = untrack(() => $state.snapshot(inputState));
|
const snapshot = $state.snapshot(input) as State;
|
||||||
writeJSON('codeStore', snapshot);
|
writeJSON(CODE_STORE_KEY, snapshot);
|
||||||
void processState(snapshot).then((processed) => {
|
void processState(snapshot).then((processed) => {
|
||||||
validatedCurrent = processed;
|
validatedCurrent = processed;
|
||||||
updateHash?.(processed.serialized);
|
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
|
// All internal reads should be done via validatedState, but it should not be
|
||||||
// persisted/shared externally.
|
// persisted/shared externally.
|
||||||
export const validatedState = {
|
export const validatedState = {
|
||||||
@@ -274,27 +288,32 @@ export const sanitizeConfig = (config: string | MermaidConfig) => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
export const loadState = (data: string): void => {
|
export const loadState = (data: string): void => {
|
||||||
let state: State;
|
|
||||||
console.log(`Loading '${data}'`);
|
console.log(`Loading '${data}'`);
|
||||||
|
update((state) => {
|
||||||
|
let next: State;
|
||||||
try {
|
try {
|
||||||
state = deserializeState(data);
|
next = deserializeState(data);
|
||||||
state.mermaid = sanitizeConfig(state.mermaid || defaultState.mermaid);
|
next.mermaid = sanitizeConfig(next.mermaid || defaultState.mermaid);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
state = untrack(() => $state.snapshot(inputState));
|
next = $state.snapshot(state) as State;
|
||||||
if (data) {
|
if (data) {
|
||||||
console.error('Init error', error);
|
console.error('Init error', error);
|
||||||
state.code = urlParseFailedState;
|
next.code = urlParseFailedState;
|
||||||
state.mermaid = defaultState.mermaid;
|
next.mermaid = defaultState.mermaid;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
updateCodeStore(state);
|
applyPartial(state, next);
|
||||||
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
let renderCount = 0;
|
let renderCount = 0;
|
||||||
export const updateCodeStore = (newState: Partial<State>): void => {
|
const applyPartial = (state: State, newState: Partial<State>): void => {
|
||||||
renderCount++;
|
renderCount++;
|
||||||
Object.assign(inputState, newState, { renderCount });
|
Object.assign(state, newState, { renderCount });
|
||||||
persistAndProcess();
|
};
|
||||||
|
|
||||||
|
export const updateCodeStore = (newState: Partial<State>): void => {
|
||||||
|
update((state) => applyPartial(state, newState));
|
||||||
};
|
};
|
||||||
|
|
||||||
export const updateCode = (
|
export const updateCode = (
|
||||||
@@ -306,13 +325,14 @@ export const updateCode = (
|
|||||||
): void => {
|
): void => {
|
||||||
errorDebug();
|
errorDebug();
|
||||||
|
|
||||||
|
update((state) => {
|
||||||
if (resetPanZoom) {
|
if (resetPanZoom) {
|
||||||
inputState.pan = undefined;
|
state.pan = undefined;
|
||||||
inputState.zoom = undefined;
|
state.zoom = undefined;
|
||||||
}
|
}
|
||||||
inputState.code = code;
|
state.code = code;
|
||||||
inputState.updateDiagram = updateDiagram;
|
state.updateDiagram = updateDiagram;
|
||||||
persistAndProcess();
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
export const updateConfig = (config: string): void => {
|
export const updateConfig = (config: string): void => {
|
||||||
@@ -320,25 +340,27 @@ export const updateConfig = (config: string): void => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
export const toggleDarkTheme = (dark: boolean): void => {
|
export const toggleDarkTheme = (dark: boolean): void => {
|
||||||
const config = JSON.parse(untrack(() => inputState.mermaid)) as MermaidConfig;
|
update((state) => {
|
||||||
|
const config = JSON.parse(state.mermaid) as MermaidConfig;
|
||||||
if (!config.theme || ['dark', 'default'].includes(config.theme)) {
|
if (!config.theme || ['dark', 'default'].includes(config.theme)) {
|
||||||
config.theme = dark ? 'dark' : 'default';
|
config.theme = dark ? 'dark' : 'default';
|
||||||
}
|
}
|
||||||
inputState.mermaid = formatJSON(config);
|
state.mermaid = formatJSON(config);
|
||||||
persistAndProcess();
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
// Replaces the whole input state (e.g. when restoring a history entry),
|
// Replaces the whole input state (e.g. when restoring a history entry),
|
||||||
// dropping keys the next state does not define.
|
// dropping keys the next state does not define.
|
||||||
export const replaceInputState = (next: State): void => {
|
export const replaceInputState = (next: State): void => {
|
||||||
for (const key of untrack(() => Object.keys(inputState))) {
|
update((state) => {
|
||||||
|
for (const key of Object.keys(state)) {
|
||||||
if (!(key in next)) {
|
if (!(key in next)) {
|
||||||
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- full-replace semantics
|
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- full-replace semantics
|
||||||
delete (inputState as unknown as Record<string, unknown>)[key];
|
delete (state as unknown as Record<string, unknown>)[key];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Object.assign(inputState, next);
|
Object.assign(state, next);
|
||||||
persistAndProcess();
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
export const initURLSubscription = (): void => {
|
export const initURLSubscription = (): void => {
|
||||||
@@ -349,5 +371,5 @@ export const initURLSubscription = (): void => {
|
|||||||
};
|
};
|
||||||
|
|
||||||
export const verifyState = (): void => {
|
export const verifyState = (): void => {
|
||||||
updateCodeStore(untrack(() => inputState.panZoom) ? {} : { panZoom: true });
|
update((state) => applyPartial(state, state.panZoom ? {} : { panZoom: true }));
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -33,6 +33,8 @@ export default defineConfig({
|
|||||||
envPrefix: 'MERMAID_',
|
envPrefix: 'MERMAID_',
|
||||||
server: { port: 3000, host: true },
|
server: { port: 3000, host: true },
|
||||||
preview: { 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: {
|
test: {
|
||||||
environment: 'jsdom',
|
environment: 'jsdom',
|
||||||
// in-source testing
|
// in-source testing
|
||||||
|
|||||||
Reference in New Issue
Block a user