Fix upload wizard tab crash: dispatch must not track the model signal
createUploadController runs an effect() that calls dispatch. store.ts dispatch was `model.set(update(model(), msg))` — the reactive model() read made the effect depend on its own write and re-schedule forever, livelocking the main thread. Angular's NG0103 guard doesn't cover effect self-rescheduling, so no error was thrown; Firefox just killed the unresponsive tab. Only /registreren and /herregistratie (which mount the upload controller) were affected. dispatch now uses model.update((m) => update(m, msg)) — the current value is read untracked, so no effect can loop on its own dispatch. Hardens all wizard stores. Adds a regression spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
32
src/app/shared/application/store.spec.ts
Normal file
32
src/app/shared/application/store.spec.ts
Normal file
@@ -0,0 +1,32 @@
|
|||||||
|
import { ApplicationRef, effect } from '@angular/core';
|
||||||
|
import { TestBed } from '@angular/core/testing';
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
import { createStore } from './store';
|
||||||
|
|
||||||
|
describe('createStore', () => {
|
||||||
|
it('applies the pure update on dispatch', () => {
|
||||||
|
const store = createStore(0, (n: number, m: number) => n + m);
|
||||||
|
store.dispatch(5);
|
||||||
|
store.dispatch(3);
|
||||||
|
expect(store.model()).toBe(8);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression: an effect that dispatches must NOT re-run because of its own write.
|
||||||
|
// dispatch used to read `model()` reactively (`set(update(model(), msg))`), so an
|
||||||
|
// effect calling dispatch subscribed to `model` and looped forever, livelocking the
|
||||||
|
// main thread (crashed the upload wizards). With `.update` the read is untracked.
|
||||||
|
it('dispatch from inside an effect does not self-loop', () => {
|
||||||
|
const store = createStore(0, (n: number, _m: 'inc') => n + 1);
|
||||||
|
let runs = 0;
|
||||||
|
TestBed.runInInjectionContext(() => {
|
||||||
|
effect(() => {
|
||||||
|
runs++;
|
||||||
|
if (runs < 100) store.dispatch('inc'); // bounded so the buggy version can't hang the test
|
||||||
|
});
|
||||||
|
});
|
||||||
|
TestBed.inject(ApplicationRef).tick(); // flush effects
|
||||||
|
|
||||||
|
expect(runs).toBe(1); // effect ran once; its own dispatch did not retrigger it
|
||||||
|
expect(store.model()).toBe(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -24,6 +24,10 @@ export function createStore<Model, Msg>(
|
|||||||
const model = signal(init);
|
const model = signal(init);
|
||||||
return {
|
return {
|
||||||
model: model.asReadonly(),
|
model: model.asReadonly(),
|
||||||
dispatch: (msg) => model.set(update(model(), msg)),
|
// Use `.update` (raw current value, no tracked read) not `set(update(model(), …))`:
|
||||||
|
// dispatch is a command and must never subscribe its caller to `model`. Reading
|
||||||
|
// `model()` here inside an effect that also dispatches makes the effect depend on
|
||||||
|
// its own write and livelock the main thread (crashed the upload wizards).
|
||||||
|
dispatch: (msg) => model.update((m) => update(m, msg)),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user