Repository navigation
fix(pwa): flush pending state before a forced SW-update reload (DA-02) #517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
812be32
fix(pwa): flush pending state before a forced SW-update reload (DA-02)
qnbs 8fb9c2b
fix(pwa): close DA-02 review-wave gaps in the flush-before-reload fix
qnbs 6868b91
fix(pwa): close DA-02's second review wave (codex P1/P2)
qnbs 75802ef
docs(pwa): correct DA-02 comments and PR description to match actual …
qnbs daf44e1
fix(pwa): close DA-02's third review wave — real implementation bugs
qnbs 3eec3da
fix(pwa): bound the pre-reload flush wait (DA-02, codex)
qnbs 21a9dfc
docs(pwa): conform QNBS-v3 comments to the literal repo format
qnbs 67328f9
fix(storage): dedupe concurrent auto-snapshot attempts (DA-02, codex)
qnbs 8d7f4ab
fix(pwa): compare only versionControl's persisted fields (DA-02, codex)
qnbs 1e6c418
fix(pwa): wait for coordinator drain, add real-IDB round trip (DA-02,…
qnbs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| // QNBS-v3 (DA-02): proves controllerchange flushes the latest visible-tab state before reloading. | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| vi.mock('../../services/logger', async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import('../../services/logger')>(); | ||
| return { ...actual, logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } }; | ||
| }); | ||
|
|
||
| const { mockFlushPersistedState } = vi.hoisted(() => ({ mockFlushPersistedState: vi.fn() })); | ||
| vi.mock('../../app/persistedStateFlush', () => ({ flushPersistedState: mockFlushPersistedState })); | ||
|
|
||
| import { appStoreRef } from '../../app/storeRef'; | ||
| import { registerServiceWorker } from '../../register-sw'; | ||
|
|
||
| async function flushMicrotasks(): Promise<void> { | ||
| await new Promise((resolve) => setTimeout(resolve, 0)); | ||
| } | ||
|
|
||
| function setVisibility(value: 'visible' | 'hidden'): void { | ||
| Object.defineProperty(document, 'visibilityState', { value, configurable: true }); | ||
| } | ||
|
|
||
| describe('register-sw — controllerchange flush-then-reload (DA-02)', () => { | ||
| let controllerChangeHandler: (() => void) | undefined; | ||
| let reloadSpy: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| controllerChangeHandler = undefined; | ||
| setVisibility('visible'); | ||
|
|
||
| for (const key of ['__TAURI_INTERNALS__', '__TAURI__', '__TAURI_METADATA__']) { | ||
| delete (window as unknown as Record<string, unknown>)[key]; | ||
| } | ||
|
|
||
| reloadSpy = vi.fn(); | ||
| Object.defineProperty(window, 'location', { | ||
| value: { ...window.location, reload: reloadSpy }, | ||
| writable: true, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| const fakeRegistration = { | ||
| scope: '/', | ||
| installing: null, | ||
| waiting: null, | ||
| addEventListener: vi.fn(), | ||
| }; | ||
|
|
||
| Object.defineProperty(navigator, 'serviceWorker', { | ||
| value: { | ||
| register: vi.fn().mockResolvedValue(fakeRegistration), | ||
| controller: {}, | ||
| addEventListener: vi.fn((type: string, handler: () => void) => { | ||
| if (type === 'controllerchange') controllerChangeHandler = handler; | ||
| }), | ||
| }, | ||
| writable: true, | ||
| configurable: true, | ||
| }); | ||
|
|
||
| // QNBS-v3: a stable reference — a fresh object each call would never satisfy the "unchanged" stop condition. | ||
| const stableState = { fake: 'state' }; | ||
| appStoreRef.current = { | ||
|
qnbs marked this conversation as resolved.
Resolved
|
||
| getState: () => stableState as never, | ||
| dispatch: vi.fn() as never, | ||
| }; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| appStoreRef.current = null; | ||
| // @ts-expect-error — test-only cleanup of a property this suite defines itself. | ||
| delete navigator.serviceWorker; | ||
| setVisibility('visible'); | ||
| }); | ||
|
|
||
| it('flushes pending state and reloads, in that order, when the visible tab takes control', async () => { | ||
| mockFlushPersistedState.mockResolvedValue(undefined); | ||
| await registerServiceWorker(); | ||
| expect(controllerChangeHandler).toBeTypeOf('function'); | ||
|
|
||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| const flushOrder = mockFlushPersistedState.mock.invocationCallOrder[0] as number; | ||
| const reloadOrder = reloadSpy.mock.invocationCallOrder[0] as number; | ||
| expect(flushOrder).toBeLessThan(reloadOrder); | ||
|
qnbs marked this conversation as resolved.
Resolved
|
||
| }); | ||
|
|
||
| it('reloads even when the flush fails, rather than staying on a bundle whose old cache is already pruned', async () => { | ||
| mockFlushPersistedState.mockRejectedValue(new Error('IDB write failed')); | ||
| await registerServiceWorker(); | ||
|
|
||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('ignores a second controllerchange event (single-flight)', async () => { | ||
| mockFlushPersistedState.mockResolvedValue(undefined); | ||
| await registerServiceWorker(); | ||
|
|
||
| controllerChangeHandler?.(); | ||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('still reloads when no store is mounted yet (defensive null-guard, nothing to flush)', async () => { | ||
| appStoreRef.current = null; | ||
| await registerServiceWorker(); | ||
|
|
||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).not.toHaveBeenCalled(); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| // QNBS-v3 (codex P1): a hidden tab's state can't be fresher than what's already persisted — only the visible tab flushes. | ||
| it('defers reload while the tab is hidden, then reloads once visible without flushing again', async () => { | ||
| mockFlushPersistedState.mockResolvedValue(undefined); | ||
| await registerServiceWorker(); | ||
| setVisibility('hidden'); | ||
|
|
||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).not.toHaveBeenCalled(); | ||
| expect(reloadSpy).not.toHaveBeenCalled(); | ||
|
|
||
| setVisibility('visible'); | ||
| document.dispatchEvent(new Event('visibilitychange')); | ||
| await flushMicrotasks(); | ||
|
|
||
| // QNBS-v3 (codex): index.tsx's own visibilitychange listener already flushed this tab when it went hidden — flushing its possibly-stale copy again here could clobber a fresher write from another tab. | ||
| expect(mockFlushPersistedState).not.toHaveBeenCalled(); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| // QNBS-v3 (CodeAnt/codex): a single snapshot could miss an edit made while the async write is still in flight. | ||
| it('re-flushes with the latest state when it changes during the pending flush, before reloading', async () => { | ||
| const stateA = { v: 'a' }; | ||
| const stateB = { v: 'b' }; | ||
| // 1st getState(): stateA. 2nd (after flush #1): stateB (changed — retry). 3rd (after flush #2): stateB (stable — stop). | ||
| const getStateMock = vi.fn().mockReturnValueOnce(stateA).mockReturnValueOnce(stateB).mockReturnValue(stateB); | ||
| appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; | ||
| mockFlushPersistedState.mockResolvedValue(undefined); | ||
|
|
||
| await registerServiceWorker(); | ||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| expect(mockFlushPersistedState).toHaveBeenCalledTimes(2); | ||
| expect(mockFlushPersistedState).toHaveBeenNthCalledWith(1, stateA); | ||
| expect(mockFlushPersistedState).toHaveBeenNthCalledWith(2, stateB); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| const lastFlushOrder = mockFlushPersistedState.mock.invocationCallOrder[1] as number; | ||
| const reloadOrder = reloadSpy.mock.invocationCallOrder[0] as number; | ||
| expect(lastFlushOrder).toBeLessThan(reloadOrder); | ||
| }); | ||
|
|
||
| // QNBS-v3 (codex P2): the retry loop can exhaust its budget while state keeps changing — one guaranteed final flush must still capture whatever's freshest, not silently drop it. | ||
| it('performs one final guaranteed flush of the freshest state after exhausting the retry budget', async () => { | ||
| const states = Array.from({ length: 7 }, (_, i) => ({ v: i })); | ||
| const getStateMock = vi.fn(); | ||
| for (const s of states) getStateMock.mockReturnValueOnce(s); | ||
| appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; | ||
| mockFlushPersistedState.mockResolvedValue(undefined); | ||
|
|
||
| await registerServiceWorker(); | ||
| controllerChangeHandler?.(); | ||
| await flushMicrotasks(); | ||
|
|
||
| // 5 in-loop attempts (states[0..4]) + 1 guaranteed final flush of the freshest state (states[6]). | ||
| expect(mockFlushPersistedState).toHaveBeenCalledTimes(6); | ||
| expect(mockFlushPersistedState).toHaveBeenNthCalledWith(6, states[6]); | ||
| expect(reloadSpy).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.