fix(client): park the detail store when its last viewer leaves
The per-session DetailStore map is page-lifetime, so a store whose Context tab, sidebar panel, or /context modal had gone kept trailing its own read schedule: a broken detail route was re-POSTed every 2.4s for the rest of the page's life with nobody reading the answer (measured on a real session: 2 extra POSTs in the 3s after unmount, unbounded). The last unsubscribe now parks the store: the armed window is cleared and no read is armed while nobody subscribes. The cached payload is kept, so a re-open still renders instantly (measured: identical rendering digest and no refetch on remount); a resume with nothing cached clears the ledger and the old backoff, so the mount's own request(rev) reads again instead of sticking on the retry note.
This commit is contained in:
1 parent
e1a6655496
commit
45e169749f
2 files changed
+140
-9
No files matched your search
@@ -128,6 +128,14 @@ export class DetailStore {
|
||||
private inFlight = false
|
||||
private timer: ReturnType<typeof setTimeout> | null = null
|
||||
private failures = 0
|
||||
/**
|
||||
* No view holds this store (the last subscriber left). A parked store keeps
|
||||
* its cached payload so a re-open renders instantly, but arms no read: the
|
||||
* map is page-lifetime, so a store whose view is gone would otherwise keep
|
||||
* trailing a broken detail route (or a moved rev) forever, with nobody
|
||||
* reading the answer.
|
||||
*/
|
||||
private parked = false
|
||||
private readonly listeners = new Set<() => void>()
|
||||
private snap: DetailSnap = EMPTY_SNAP
|
||||
|
||||
@@ -138,7 +146,23 @@ export class DetailStore {
|
||||
|
||||
readonly subscribe = (fn: () => void): (() => void) => {
|
||||
this.listeners.add(fn)
|
||||
return () => this.listeners.delete(fn)
|
||||
if (this.parked) {
|
||||
this.parked = false
|
||||
// Resume: with nothing cached (a failure or an absence), the head the
|
||||
// view then requests may equal the cursor the failed read already left
|
||||
// behind — it would be swallowed as "wanted". Clear the ledger so the
|
||||
// mount's own request(rev) reads again instead of sticking on the note,
|
||||
// and drop the old backoff so the fresh attempt waits out the base
|
||||
// window rather than a parked failure's doubled one.
|
||||
if (this.detail === null) {
|
||||
this.resetLedger()
|
||||
this.failures = 0
|
||||
}
|
||||
}
|
||||
return () => {
|
||||
this.listeners.delete(fn)
|
||||
if (this.listeners.size === 0) this.park()
|
||||
}
|
||||
}
|
||||
|
||||
readonly getSnapshot = (): DetailSnap => this.snap
|
||||
@@ -150,13 +174,7 @@ export class DetailStore {
|
||||
* no longer comparable, so the ledger resets and refetches.
|
||||
*/
|
||||
request(rev: number): void {
|
||||
if (rev < this.lastHeadRev) {
|
||||
this.generation++
|
||||
this.acceptedRev = -1
|
||||
this.wantedRev = -1
|
||||
this.detail = null
|
||||
this.failed = false
|
||||
}
|
||||
if (rev < this.lastHeadRev) this.resetLedger()
|
||||
this.lastHeadRev = rev
|
||||
if (rev <= this.acceptedRev || rev <= this.wantedRev) return
|
||||
this.wantedRev = rev
|
||||
@@ -174,8 +192,31 @@ export class DetailStore {
|
||||
if (!this.inFlight) void this.fire()
|
||||
}
|
||||
|
||||
/** Drop the served payload and its cursors — a host refold, or a resume with nothing cached. */
|
||||
private resetLedger(): void {
|
||||
this.generation++
|
||||
this.acceptedRev = -1
|
||||
this.wantedRev = -1
|
||||
this.detail = null
|
||||
this.failed = false
|
||||
// Drop the snapshot's own payload reference too: it is the second place a
|
||||
// released detail could stay reachable from.
|
||||
this.snap = EMPTY_SNAP
|
||||
}
|
||||
|
||||
/** The last viewer left: clear the pending read (the cached payload stays). */
|
||||
private park(): void {
|
||||
this.parked = true
|
||||
if (this.timer !== null) {
|
||||
clearTimeout(this.timer)
|
||||
this.timer = null
|
||||
}
|
||||
this.emit()
|
||||
}
|
||||
|
||||
private schedule(): void {
|
||||
if (this.timer !== null) return
|
||||
// Parked: nobody reads the answer, so never arm one.
|
||||
if (this.parked || this.timer !== null) return
|
||||
this.timer = setTimeout(() => {
|
||||
this.timer = null
|
||||
void this.fire()
|
||||
|
||||
@@ -215,6 +215,96 @@ describe('DetailStore', () => {
|
||||
assert.equal(seen.length, 0)
|
||||
})
|
||||
|
||||
test('a parked store arms no read (the last unsubscribe drops the pending window)', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { fetcher, calls } = scriptedFetcher([detail(1)])
|
||||
const store = new DetailStore(fetcher, 10)
|
||||
const off = store.subscribe(() => {})
|
||||
store.request(1)
|
||||
assert.equal(store.getSnapshot().pending, true, 'the debounce window is armed')
|
||||
off()
|
||||
assert.equal(store.getSnapshot().pending, false, 'parking drops the pending window')
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
assert.equal(calls.length, 0, 'a parked store never reaches the fetcher')
|
||||
// A further rev while still parked arms nothing either.
|
||||
store.request(2)
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
assert.equal(calls.length, 0)
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
test('a read settling after the last unsubscribe does not re-arm', async () => {
|
||||
vi.useFakeTimers()
|
||||
let reject!: (error: Error) => void
|
||||
let calls = 0
|
||||
const store = new DetailStore(() => {
|
||||
calls++
|
||||
return new Promise<ContextTimelineDetail | null>((_resolve, rej) => { reject = rej })
|
||||
}, 10)
|
||||
const off = store.subscribe(() => {})
|
||||
store.request(1)
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
assert.equal(calls, 1, 'the read is in flight')
|
||||
off()
|
||||
reject(new Error('offline'))
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
assert.equal(calls, 1, 'no retry is armed once nobody is subscribed')
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
test('a store parked without a payload re-reads on the next subscribe', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { fetcher, calls } = scriptedFetcher([new Error('offline'), detail(1)])
|
||||
const store = new DetailStore(fetcher, 10)
|
||||
const off = store.subscribe(() => {})
|
||||
store.request(1)
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
assert.equal(calls.length, 1)
|
||||
assert.equal(store.getSnapshot().failed, true)
|
||||
off()
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
assert.equal(calls.length, 1, 'the parked retry is dropped')
|
||||
store.subscribe(() => {})
|
||||
// The SAME head rev must read again: the failed read already marked it
|
||||
// "wanted", which would otherwise swallow the resumed store's request.
|
||||
store.request(1)
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
assert.equal(calls.length, 2, 'the resumed store reads instead of sticking on the note')
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
test('a parked store with a cached payload resumes without a read', async () => {
|
||||
const { fetcher, calls } = scriptedFetcher([detail(1)])
|
||||
const store = new DetailStore(fetcher, 0)
|
||||
const off = store.subscribe(() => {})
|
||||
store.request(1)
|
||||
await until(() => store.getSnapshot().detail?.rev === 1, 'the first read never landed')
|
||||
off()
|
||||
store.subscribe(() => {})
|
||||
store.request(1)
|
||||
await settle()
|
||||
assert.equal(calls.length, 1, 'the cached payload still covers the rev')
|
||||
})
|
||||
|
||||
test('a shared store parks only when its last subscriber leaves', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { fetcher, calls } = scriptedFetcher([detail(1)])
|
||||
const store = new DetailStore(fetcher, 10)
|
||||
// The tab and the /context modal share one store per session.
|
||||
const offTab = store.subscribe(() => {})
|
||||
const offModal = store.subscribe(() => {})
|
||||
store.request(1)
|
||||
offTab()
|
||||
await vi.advanceTimersByTimeAsync(10)
|
||||
assert.equal(calls.length, 1, 'the remaining subscriber still gets its read')
|
||||
offModal()
|
||||
store.request(2)
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
assert.equal(calls.length, 1, 'the parked store arms nothing')
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
test('a rev bump during the in-flight read re-reads after settle (trailing edge)', async () => {
|
||||
let gate!: () => void
|
||||
const first = new Promise<ContextTimelineDetail | null>(resolve => { gate = () => resolve(detail(1)) })
|
||||
|
||||
Reference in new issue
Block a user