Skip to content

Commit 35642f1

Browse files
committed
fix(core): two resource cache runtime edge cases
Fix 1 — in-flight promise after inactive eviction must not mutate detached entries. Add ownership check (entries.get(key) !== entry) before applying success/failure results, calling staleNotify(), or starting staleTime/cacheTime timers. Fix 2 — switching active key to an already in-flight entry must sync public state (syncFromEntry) before dedupe early return. Previously the dedupe path returned the in-flight promise without updating currentStatus/currentValue/currentError, leaving public getters showing the previous key's state. Add tests: - detached promise resolution does not notify subscribers - detached promise resolution does not start timers - switching to in-flight key syncs state before dedupe return
1 parent e8ecbfa commit 35642f1

2 files changed

Lines changed: 140 additions & 0 deletions

File tree

packages/core/src/core.test.ts

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4611,4 +4611,140 @@ describe("resource cache phase 3 - cacheTime", () => {
46114611
expect(resource.status).toBe("ready")
46124612
expect(resource.value).toBe("data-a")
46134613
})
4614+
4615+
it("resolving detached in-flight promise does not notify subscribers", async () => {
4616+
let resolveLoad!: (v: string) => void
4617+
const deferred = new Promise<string>(resolve => { resolveLoad = resolve })
4618+
4619+
const resource = createResourceNode("team", "team", async (ctx: { id: string }) => {
4620+
if (ctx.id === "a") return deferred
4621+
return `data-${ctx.id}`
4622+
}, true, {
4623+
key: (ctx: { id: string }) => ctx.id,
4624+
cacheTime: 10,
4625+
})
4626+
4627+
// Track node-level and stale notifications
4628+
let notifyCount = 0
4629+
const unsub = resource.subscribe(() => { notifyCount++ })
4630+
let staleNotifyCount = 0
4631+
const staleUnsub = resource.stale.subscribe(() => { staleNotifyCount++ })
4632+
4633+
// Start loading "a" with a deferred promise
4634+
const loadAPromise = resource.load({ id: "a" })
4635+
await new Promise(r => setTimeout(r, 5))
4636+
4637+
// Invalidate "a" and switch to "b" — "a" becomes inactive
4638+
resource.invalidate()
4639+
await resource.load({ id: "b" })
4640+
4641+
// Wait for "a"'s cacheTime to fire (inactive eviction)
4642+
await new Promise(r => setTimeout(r, 20))
4643+
4644+
const notifyBefore = notifyCount
4645+
const staleBefore = staleNotifyCount
4646+
4647+
// Resolve "a"'s deferred — entry is detached, must be a no-op
4648+
resolveLoad("a-data")
4649+
await loadAPromise
4650+
4651+
expect(notifyCount).toBe(notifyBefore)
4652+
expect(staleNotifyCount).toBe(staleBefore)
4653+
4654+
unsub()
4655+
staleUnsub()
4656+
})
4657+
4658+
it("resolving detached in-flight promise does not start staleTime or cacheTime timers", async () => {
4659+
let resolveLoad!: (v: string) => void
4660+
const deferred = new Promise<string>(resolve => { resolveLoad = resolve })
4661+
let loadCountForA = 0
4662+
4663+
const resource = createResourceNode("team", "team", async (ctx: { id: string }) => {
4664+
if (ctx.id === "a") {
4665+
loadCountForA++
4666+
if (loadCountForA === 1) return deferred
4667+
}
4668+
return `data-${ctx.id}`
4669+
}, true, {
4670+
key: (ctx: { id: string }) => ctx.id,
4671+
cacheTime: 20,
4672+
})
4673+
4674+
// Start loading "a" with a deferred promise
4675+
const loadAPromise = resource.load({ id: "a" })
4676+
await new Promise(r => setTimeout(r, 5))
4677+
4678+
// Invalidate "a" and switch to "b" — "a" becomes inactive with cacheTime timer
4679+
resource.invalidate()
4680+
await resource.load({ id: "b" })
4681+
expect(resource.status).toBe("ready")
4682+
expect(resource.value).toBe("data-b")
4683+
4684+
// Wait for "a"'s cacheTime to fire (inactive eviction)
4685+
await new Promise(r => setTimeout(r, 30))
4686+
4687+
// Resolve "a"'s deferred — entry is detached, no timers should start
4688+
resolveLoad("a-data")
4689+
await loadAPromise
4690+
4691+
// "b" is still healthy (no side effects from detached resolution)
4692+
expect(resource.status).toBe("ready")
4693+
expect(resource.value).toBe("data-b")
4694+
4695+
// "a" was not resurrected — loading it creates a fresh entry
4696+
await resource.load({ id: "a" })
4697+
expect(resource.value).toBe("data-a")
4698+
4699+
resource.dispose()
4700+
})
4701+
4702+
it("switching to a key with in-flight promise syncs state before dedupe return", async () => {
4703+
let resolveA!: (v: string) => void
4704+
const deferredA = new Promise<string>(resolve => { resolveA = resolve })
4705+
let resolveB!: (v: string) => void
4706+
const deferredB = new Promise<string>(resolve => { resolveB = resolve })
4707+
4708+
let callCount = 0
4709+
const resource = createResourceNode("team", "team", async (ctx: { id: string }) => {
4710+
callCount++
4711+
if (ctx.id === "a") return deferredA
4712+
return deferredB
4713+
}, true, {
4714+
key: (ctx: { id: string }) => ctx.id,
4715+
deduplicate: true,
4716+
})
4717+
4718+
// 1. Start load for key A and leave it pending
4719+
resource.load({ id: "a" })
4720+
await new Promise(r => setTimeout(r, 5))
4721+
expect(resource.status).toBe("pending")
4722+
4723+
// 2. Switch to key B and complete it
4724+
const loadB = resource.load({ id: "b" })
4725+
resolveB("b-data")
4726+
await loadB
4727+
expect(resource.status).toBe("ready")
4728+
expect(resource.value).toBe("b-data")
4729+
expect(callCount).toBe(2)
4730+
4731+
// 3. Call load for key A again while A is still in-flight (dedupe)
4732+
const loadA2 = resource.load({ id: "a" })
4733+
4734+
// 4. The node immediately shows key A pending, not key B ready
4735+
expect(resource.status).toBe("pending")
4736+
expect(resource.value).toBeUndefined()
4737+
4738+
// 5. When A resolves, node shows key A value
4739+
resolveA("a-data")
4740+
await loadA2
4741+
expect(resource.status).toBe("ready")
4742+
expect(resource.value).toBe("a-data")
4743+
4744+
// Regression: same-key dedup still calls loader once
4745+
expect(callCount).toBe(2)
4746+
4747+
// Regression: different-key loads ran independently (A=call 1, B=call 2)
4748+
// Already verified by callCount === 2
4749+
})
46144750
})

packages/core/src/resource.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ export function createResourceNode<TValue, TServices extends object = DefaultScr
278278

279279
function executeLoad(context?: ResourceLoadContext<TServices>): Promise<void> {
280280
const key = resolveKey(context)
281+
const prevActiveKey = _activeKey
281282
_activeKey = key
282283

283284
let entry = entries.get(key)
@@ -289,6 +290,7 @@ export function createResourceNode<TValue, TServices extends object = DefaultScr
289290
_clearEntryCacheTimer(entry)
290291

291292
if (shouldDeduplicate && entry.inFlightPromise) {
293+
if (key !== prevActiveKey) syncFromEntry(entry)
292294
return entry.inFlightPromise
293295
}
294296

@@ -308,6 +310,7 @@ export function createResourceNode<TValue, TServices extends object = DefaultScr
308310
const result = await Promise.resolve(
309311
(loader as (ctx: ResourceLoadContext<TServices>) => TValue | Promise<TValue>)(loadContext)
310312
)
313+
if (entries.get(key) !== entry) return
311314
entry!.value = result
312315
entry!.status = "ready"
313316
entry!.stale = false
@@ -316,6 +319,7 @@ export function createResourceNode<TValue, TServices extends object = DefaultScr
316319
staleNotify()
317320
_startEntryStaleTimer(entry!, key)
318321
} catch (e: unknown) {
322+
if (entries.get(key) !== entry) return
319323
entry!.error = e
320324
entry!.status = "failed"
321325
entry!.stale = false

0 commit comments

Comments
 (0)