From 3d2cc8e7e4ffdecec09f6440a753fd479d62a053 Mon Sep 17 00:00:00 2001 From: Sean Doherty Date: Sun, 17 May 2026 06:31:32 -0500 Subject: [PATCH 1/2] separate background fetch state from values --- src/index.ts | 312 +++++++++++++++---------- tap-snapshots/test/tracing.ts.test.cjs | 30 +++ test/fetch.ts | 47 ++-- 3 files changed, 251 insertions(+), 138 deletions(-) diff --git a/src/index.ts b/src/index.ts index 09ca0de..d7a8b5e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -111,11 +111,7 @@ class Stack { /** * Promise representing an in-progress {@link LRUCache#fetch} call */ -export type BackgroundFetch = Promise & { - __returned: BackgroundFetch | undefined - __abortController: AbortController - __staleWhileFetching: V | undefined -} +export type BackgroundFetch = Promise export type DisposeTask = [ value: V, @@ -1274,7 +1270,10 @@ export class LRUCache { #calculatedSize: LRUCache.Size #keyMap: Map #keyList: (K | undefined)[] - #valList: (V | BackgroundFetch | undefined)[] + #valList: (V | undefined)[] + #fetchList?: (BackgroundFetch | undefined)[] + #abortControllerList?: (AbortController | undefined)[] + #returnedFetch?: NumberArray #next: NumberArray #prev: NumberArray #head: Index @@ -1314,6 +1313,9 @@ export class LRUCache { keyMap: c.#keyMap as Map, keyList: c.#keyList, valList: c.#valList, + get fetchList() { + return c.#fetchList + }, next: c.#next, prev: c.#prev, get head() { @@ -1324,7 +1326,6 @@ export class LRUCache { }, free: c.#free, // methods - isBackgroundFetch: (p: unknown) => c.#isBackgroundFetch(p), backgroundFetch: ( k: K, index: number | undefined, @@ -1702,11 +1703,6 @@ export class LRUCache { sizes[index] = 0 } this.#requireSize = (k, v, size, sizeCalculation) => { - // provisionally accept background fetches. - // actual value size will be checked when they return. - if (this.#isBackgroundFetch(v)) { - return 0 - } if (!isPosInt(size)) { if (sizeCalculation) { if (typeof sizeCalculation !== 'function') { @@ -1759,13 +1755,13 @@ export class LRUCache { #requireSize: ( k: K, - v: V | BackgroundFetch, + v: V, size?: LRUCache.Size, sizeCalculation?: LRUCache.SizeCalculator, status?: LRUCache.Status, ) => LRUCache.Size = ( _k: K, - _v: V | BackgroundFetch, + _v: V, size?: LRUCache.Size, sizeCalculation?: LRUCache.SizeCalculator, ) => { @@ -1775,7 +1771,58 @@ export class LRUCache { ) } return 0 - }; + } + + #setBackgroundFetch( + index: Index, + bf: BackgroundFetch, + ac: AbortController, + ) { + if (!this.#fetchList) { + this.#fetchList = Array.from({ length: this.#max }).fill( + undefined, + ) as (BackgroundFetch | undefined)[] + } + if (!this.#abortControllerList) { + this.#abortControllerList = Array.from({ length: this.#max }).fill( + undefined, + ) as (AbortController | undefined)[] + } + if (!this.#returnedFetch) { + this.#returnedFetch = this.#max ? new Uint8Array(this.#max) : [] + } + this.#fetchList[index] = bf + this.#abortControllerList[index] = ac + this.#returnedFetch[index] = 0 + } + + #clearBackgroundFetch(index: Index) { + if (this.#fetchList) { + this.#fetchList[index] = undefined + } + if (this.#abortControllerList) { + this.#abortControllerList[index] = undefined + } + } + + #abortBackgroundFetch( + index: Index, + reason: Error, + ): BackgroundFetch | undefined { + const bf = this.#fetchList?.[index] + if (bf) { + this.#abortControllerList?.[index]?.abort(reason) + this.#clearBackgroundFetch(index) + } + return bf + } + + #returnBackgroundFetch(index: Index, bf: BackgroundFetch) { + if (this.#returnedFetch) { + this.#returnedFetch[index] = 1 + } + return bf + } *#indexes({ allowStale = this.allowStale } = {}) { if (this.#size) { @@ -1823,7 +1870,7 @@ export class LRUCache { if ( this.#valList[i] !== undefined && this.#keyList[i] !== undefined && - !this.#isBackgroundFetch(this.#valList[i]) + this.#fetchList?.[i] === undefined ) { yield [this.#keyList[i], this.#valList[i]] as [K, V] } @@ -1841,7 +1888,7 @@ export class LRUCache { if ( this.#valList[i] !== undefined && this.#keyList[i] !== undefined && - !this.#isBackgroundFetch(this.#valList[i]) + this.#fetchList?.[i] === undefined ) { yield [this.#keyList[i], this.#valList[i]] } @@ -1855,7 +1902,7 @@ export class LRUCache { *keys() { for (const i of this.#indexes()) { const k = this.#keyList[i] - if (k !== undefined && !this.#isBackgroundFetch(this.#valList[i])) { + if (k !== undefined && this.#fetchList?.[i] === undefined) { yield k } } @@ -1870,7 +1917,7 @@ export class LRUCache { *rkeys() { for (const i of this.#rindexes()) { const k = this.#keyList[i] - if (k !== undefined && !this.#isBackgroundFetch(this.#valList[i])) { + if (k !== undefined && this.#fetchList?.[i] === undefined) { yield k } } @@ -1883,8 +1930,8 @@ export class LRUCache { *values() { for (const i of this.#indexes()) { const v = this.#valList[i] - if (v !== undefined && !this.#isBackgroundFetch(this.#valList[i])) { - yield this.#valList[i] as V + if (v !== undefined && this.#fetchList?.[i] === undefined) { + yield v } } } @@ -1898,8 +1945,8 @@ export class LRUCache { *rvalues() { for (const i of this.#rindexes()) { const v = this.#valList[i] - if (v !== undefined && !this.#isBackgroundFetch(this.#valList[i])) { - yield this.#valList[i] + if (v !== undefined && this.#fetchList?.[i] === undefined) { + yield v } } } @@ -1928,8 +1975,7 @@ export class LRUCache { getOptions: LRUCache.GetOptions = {}, ) { for (const i of this.#indexes()) { - const v = this.#valList[i] - const value = this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const value = this.#valList[i] if (value === undefined) continue if (fn(value, this.#keyList[i] as K, this)) { return this.#get(this.#keyList[i] as K, getOptions) @@ -1953,8 +1999,7 @@ export class LRUCache { thisp: unknown = this, ) { for (const i of this.#indexes()) { - const v = this.#valList[i] - const value = this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const value = this.#valList[i] if (value === undefined) continue fn.call(thisp, value, this.#keyList[i] as K, this) } @@ -1969,8 +2014,7 @@ export class LRUCache { thisp: unknown = this, ) { for (const i of this.#rindexes()) { - const v = this.#valList[i] - const value = this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const value = this.#valList[i] if (value === undefined) continue fn.call(thisp, value, this.#keyList[i] as K, this) } @@ -2006,11 +2050,9 @@ export class LRUCache { info(key: K): LRUCache.Entry | undefined { const i = this.#keyMap.get(key) if (i === undefined) return undefined - const v = this.#valList[i] /* c8 ignore start - this isn't tested for the info function, * but it's the same logic as found in other places. */ - const value: V | undefined = - this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const value = this.#valList[i] if (value === undefined) return undefined /* c8 ignore stop */ const entry: LRUCache.Entry = { value } @@ -2046,9 +2088,7 @@ export class LRUCache { const arr: [K, LRUCache.Entry][] = [] for (const i of this.#indexes({ allowStale: true })) { const key = this.#keyList[i] - const v = this.#valList[i] - const value: V | undefined = - this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const value = this.#valList[i] if (value === undefined || key === undefined) continue const entry: LRUCache.Entry = { value } if (this.#ttls && this.#starts) { @@ -2143,7 +2183,7 @@ export class LRUCache { #set( k: K, - v: V | BackgroundFetch | undefined, + v: V | undefined, setOptions: LRUCache.SetOptions = {}, ) { const { @@ -2161,7 +2201,7 @@ export class LRUCache { } let { noUpdateTTL = this.noUpdateTTL } = setOptions - if (status && !this.#isBackgroundFetch(v)) status.value = v + if (status) status.value = v const size = this.#requireSize( k, @@ -2205,20 +2245,21 @@ export class LRUCache { } else { // update this.#moveToTail(index) - const oldVal = this.#valList[index] as V | BackgroundFetch - if (v !== oldVal) { - if (this.#hasFetchMethod && this.#isBackgroundFetch(oldVal)) { - oldVal.__abortController.abort(new Error('replaced')) - const { __staleWhileFetching: s } = oldVal - if (s !== undefined && !noDisposeOnSet) { + const oldVal = this.#valList[index] as V | undefined + const oldFetch = this.#fetchList?.[index] + if (v !== oldVal || oldFetch) { + if (oldFetch) { + this.#abortControllerList?.[index]?.abort(new Error('replaced')) + this.#clearBackgroundFetch(index) + if (oldVal !== undefined && !noDisposeOnSet) { if (this.#hasDispose) { - this.#dispose?.(s as V, k, 'set') + this.#dispose?.(oldVal as V, k, 'set') } if (this.#hasDisposeAfter) { - this.#disposed?.push([s as V, k, 'set']) + this.#disposed?.push([oldVal as V, k, 'set']) } } - } else if (!noDisposeOnSet) { + } else if (!noDisposeOnSet && oldVal !== undefined) { if (this.#hasDispose) { this.#dispose?.(oldVal as V, k, 'set') } @@ -2231,11 +2272,7 @@ export class LRUCache { this.#valList[index] = v if (status) { status.set = 'replace' - const oldValue = - oldVal && this.#isBackgroundFetch(oldVal) ? - oldVal.__staleWhileFetching - : oldVal - if (oldValue !== undefined) status.oldValue = oldValue + if (oldVal !== undefined) status.oldValue = oldVal } } else if (status) { status.set = 'update' @@ -2271,13 +2308,12 @@ export class LRUCache { pop(): V | undefined { try { while (this.#size) { + const fetching = this.#fetchList?.[this.#head] !== undefined const val = this.#valList[this.#head] this.#evict(true) - if (this.#isBackgroundFetch(val)) { - if (val.__staleWhileFetching) { - return val.__staleWhileFetching - } - } else if (val !== undefined) { + if (fetching && val !== undefined) { + return val + } else if (!fetching && val !== undefined) { return val } } @@ -2295,10 +2331,13 @@ export class LRUCache { #evict(free: boolean) { const head = this.#head const k = this.#keyList[head] as K - const v = this.#valList[head] as V - if (this.#hasFetchMethod && this.#isBackgroundFetch(v)) { - v.__abortController.abort(new Error('evicted')) - } else if (this.#hasDispose || this.#hasDisposeAfter) { + const v = this.#valList[head] as V | undefined + const bf = this.#abortBackgroundFetch(head, new Error('evicted')) + if ( + !bf && + v !== undefined && + (this.#hasDispose || this.#hasDisposeAfter) + ) { if (this.#hasDispose) { this.#dispose?.(v, k, 'evict') } @@ -2360,10 +2399,7 @@ export class LRUCache { const index = this.#keyMap.get(k) if (index !== undefined) { const v = this.#valList[index] - if ( - this.#isBackgroundFetch(v) && - v.__staleWhileFetching === undefined - ) { + if (this.#fetchList?.[index] !== undefined && v === undefined) { return false } if (!this.#isStale(index)) { @@ -2413,7 +2449,7 @@ export class LRUCache { return undefined } const v = this.#valList[index] - const val = this.#isBackgroundFetch(v) ? v.__staleWhileFetching : v + const val = v if (status) { if (val !== undefined) { status.peek = 'hit' @@ -2431,10 +2467,12 @@ export class LRUCache { options: LRUCache.FetchOptions, context: FC, ): BackgroundFetch { - const v = index === undefined ? undefined : this.#valList[index] - if (this.#isBackgroundFetch(v)) { - return v + const existing = + index === undefined ? undefined : this.#fetchList?.[index] + if (existing) { + return existing } + const v = index === undefined ? undefined : this.#valList[index] const ac = new AbortController() const { signal } = options @@ -2448,6 +2486,7 @@ export class LRUCache { options, context, } + let bf: BackgroundFetch const cb = (v: V | undefined, updateCache = false): V | undefined => { const { aborted } = ac.signal @@ -2468,15 +2507,21 @@ export class LRUCache { return fetchFail(ac.signal.reason, proceed) } // either we didn't abort, and are still here, or we did, and ignored - const bf = p as BackgroundFetch // if nothing else has been written there but we're set to update the // cache and ignore the abort, or if it's still pending on this specific // background request, then write it to the cache. - const vl = this.#valList[index as Index] - if (vl === p || (vl === undefined && ignoreAbort && updateCache)) { + const i = index as Index + const vl = this.#valList[i] + if ( + this.#fetchList?.[i] === bf || + (this.#fetchList?.[i] === undefined && + vl === undefined && + ignoreAbort && + updateCache) + ) { if (v === undefined) { - if (bf.__staleWhileFetching !== undefined) { - this.#valList[index as Index] = bf.__staleWhileFetching + if (vl !== undefined) { + this.#clearBackgroundFetch(i) } else { this.#delete(k, 'fetch') } @@ -2503,12 +2548,15 @@ export class LRUCache { const allowStale = allowStaleAborted || options.allowStaleOnFetchRejection const noDelete = allowStale || options.noDeleteOnFetchRejection - const bf = p as BackgroundFetch - if (this.#valList[index as Index] === p) { + const i = index as Index + const staleWhileFetching = + this.#fetchList?.[i] === bf ? this.#valList[i] : v + const returned = !!this.#returnedFetch?.[i] + if (this.#fetchList?.[i] === bf) { // if we allow stale on fetch rejections, then we need to ensure that // the stale value is not removed from the cache when the fetch fails. const del = - !noDelete || (!proceed && bf.__staleWhileFetching === undefined) + !noDelete || (!proceed && staleWhileFetching === undefined) if (del) { this.#delete(k, 'fetch') } else if (!allowStaleAborted) { @@ -2516,15 +2564,15 @@ export class LRUCache { // since we are done with the promise at this point. // leave it untouched if we're still waiting for an // aborted background fetch that hasn't yet returned. - this.#valList[index as Index] = bf.__staleWhileFetching + this.#clearBackgroundFetch(i) } } if (allowStale) { - if (options.status && bf.__staleWhileFetching !== undefined) { + if (options.status && staleWhileFetching !== undefined) { options.status.returnedStale = true } - return bf.__staleWhileFetching - } else if (bf.__returned === bf) { + return staleWhileFetching + } else if (returned) { throw er } } @@ -2552,34 +2600,36 @@ export class LRUCache { } if (options.status) options.status.fetchDispatched = true - const p = new Promise(pcall).then(cb, eb) - const bf: BackgroundFetch = Object.assign(p, { - __abortController: ac, - __staleWhileFetching: v, - __returned: undefined, - }) + bf = new Promise(pcall).then(cb, eb) if (index === undefined) { - // internal, don't expose status. - this.#set(k, bf, { ...fetchOpts.options, status: undefined }) - index = this.#keyMap.get(k) + const { ttl = this.ttl } = fetchOpts.options + index = ( + this.#size === 0 ? this.#tail + : this.#free.length !== 0 ? this.#free.pop() + : this.#size === this.#max ? this.#evict(false) + : this.#size) as Index + this.#keyList[index] = k + this.#valList[index] = undefined + this.#keyMap.set(k, index) + this.#next[this.#tail] = index + this.#prev[index] = this.#tail + this.#tail = index + this.#size++ + this.#addItemSize(index, 0) + if (ttl !== 0 && !this.#ttls) { + this.#initializeTTLTracking() + } + if (this.#ttls) { + this.#setItemTTL(index, ttl) + } + this.#setBackgroundFetch(index, bf, ac) } else { - this.#valList[index] = bf + this.#setBackgroundFetch(index, bf, ac) } return bf } - #isBackgroundFetch(p: unknown): p is BackgroundFetch { - if (!this.#hasFetchMethod) return false - const b = p as BackgroundFetch - return ( - !!b && - b instanceof Promise && - b.hasOwnProperty('__staleWhileFetching') && - b.__abortController instanceof AbortController - ) - } - /** * Make an asynchronous cached fetch using the * {@link LRUCache.OptionsBase.fetchMethod} function. @@ -2761,18 +2811,20 @@ export class LRUCache { let index = this.#keyMap.get(k) if (index === undefined) { if (status) status.fetch = 'miss' - const p = this.#backgroundFetch(k, index, options, context as FC) - return (p.__returned = p) + const bf = this.#backgroundFetch(k, index, options, context as FC) + index = this.#keyMap.get(k) as Index + return this.#returnBackgroundFetch(index, bf) } else { // in cache, maybe already fetching const v = this.#valList[index] - if (this.#isBackgroundFetch(v)) { - const stale = allowStale && v.__staleWhileFetching !== undefined + const inflight = this.#fetchList?.[index] + if (inflight) { + const stale = allowStale && v !== undefined if (status) { status.fetch = 'inflight' if (stale) status.returnedStale = true } - return stale ? v.__staleWhileFetching : (v.__returned = v) + return stale ? v : this.#returnBackgroundFetch(index, inflight) } // if we force a refresh, that means do NOT serve the cached value, @@ -2790,14 +2842,21 @@ export class LRUCache { // ok, it is stale or a forced refresh, and not already fetching. // refresh the cache. - const p = this.#backgroundFetch(k, index, options, context as FC) - const hasStale = p.__staleWhileFetching !== undefined + const refreshFetch = this.#backgroundFetch( + k, + index, + options, + context as FC, + ) + const hasStale = v !== undefined const staleVal = hasStale && allowStale if (status) { status.fetch = isStale ? 'stale' : 'refresh' if (staleVal && isStale) status.returnedStale = true } - return staleVal ? p.__staleWhileFetching : (p.__returned = p) + return staleVal ? v : ( + this.#returnBackgroundFetch(index, refreshFetch) + ) } } @@ -2963,7 +3022,7 @@ export class LRUCache { return undefined } const value = this.#valList[index] - const fetching = this.#isBackgroundFetch(value) + const fetching = this.#fetchList?.[index] !== undefined if (status) this.#statusTTL(status, index) if (this.#isStale(index)) { // delete only if not an in-flight background fetch @@ -2979,24 +3038,22 @@ export class LRUCache { return undefined } if (status) status.get = 'stale-fetching' - if (allowStale && value.__staleWhileFetching !== undefined) { + if (allowStale && value !== undefined) { if (status) status.returnedStale = true - return value.__staleWhileFetching + return value } return undefined } // not stale if (status) status.get = fetching ? 'fetching' : 'hit' - // if we're currently fetching it, we don't actually have it yet - // it's not stale, which means this isn't a staleWhileRefetching. - // If it's not stale, and fetching, AND has a __staleWhileFetching - // value, then that means the user fetched with {forceRefresh:true}, - // so it's safe to return that value. + // if we're currently fetching it, an undefined value means we don't + // actually have it yet. If there is a value, then the user fetched with + // {forceRefresh:true}, so it's safe to return that stale value. this.#moveToTail(index) if (updateAgeOnGet) { this.#updateItemAge(index) } - return fetching ? value.__staleWhileFetching : value + return value } #connect(p: Index, n: Index) { @@ -3058,9 +3115,15 @@ export class LRUCache { } else { this.#removeItemSize(index) const v = this.#valList[index] - if (this.#isBackgroundFetch(v)) { - v.__abortController.abort(new Error('deleted')) - } else if (this.#hasDispose || this.#hasDisposeAfter) { + const bf = this.#abortBackgroundFetch( + index, + new Error('deleted'), + ) + if ( + !bf && + v !== undefined && + (this.#hasDispose || this.#hasDisposeAfter) + ) { if (this.#hasDispose) { this.#dispose?.(v as V, k, reason) } @@ -3105,9 +3168,8 @@ export class LRUCache { #clear(reason: LRUCache.DisposeReason) { for (const index of this.#rindexes({ allowStale: true })) { const v = this.#valList[index] - if (this.#isBackgroundFetch(v)) { - v.__abortController.abort(new Error('deleted')) - } else { + const bf = this.#abortBackgroundFetch(index, new Error('deleted')) + if (!bf && v !== undefined) { const k = this.#keyList[index] if (this.#hasDispose) { this.#dispose?.(v as V, k as K, reason) @@ -3120,6 +3182,8 @@ export class LRUCache { this.#keyMap.clear() this.#valList.fill(undefined) + this.#fetchList?.fill(undefined) + this.#abortControllerList?.fill(undefined) this.#keyList.fill(undefined) if (this.#ttls && this.#starts) { this.#ttls.fill(0) diff --git a/tap-snapshots/test/tracing.ts.test.cjs b/tap-snapshots/test/tracing.ts.test.cjs index e801ea2..1c8b523 100644 --- a/tap-snapshots/test/tracing.ts.test.cjs +++ b/tap-snapshots/test/tracing.ts.test.cjs @@ -4002,6 +4002,36 @@ Map { } ` +exports[`test/tracing.ts > TAP > test/tracing.ts > TAP > fetch initializes ttl tracking from fetch options metrics 1`] = ` +Array [] +` + +exports[`test/tracing.ts > TAP > test/tracing.ts > TAP > fetch initializes ttl tracking from fetch options traces 1`] = ` +Map { + Object { + "fetch": "miss", + "fetchDispatched": true, + "fetchResolved": true, + "fetchUpdated": true, + "key": 1, + "now": 972, + "op": "fetch", + "remainingTTL": 100, + "result": 1, + "set": "replace", + "start": 972, + "trace": true, + "ttl": 100, + "value": 1, + } => Array [ + "start", + "end", + "asyncStart", + "asyncEnd", + ], +} +` + exports[`test/tracing.ts > TAP > test/tracing.ts > TAP > fetch options, signal metrics 1`] = ` Array [ Object { diff --git a/test/fetch.ts b/test/fetch.ts index fbfd4f8..ab6fd5f 100644 --- a/test/fetch.ts +++ b/test/fetch.ts @@ -70,9 +70,12 @@ t.test('asynchronous fetching', async t => { } t.matchSnapshot(JSON.stringify(dump), 'safe to stringify dump') - t.equal(e.isBackgroundFetch(v), true) - t.equal(e.backgroundFetch('key', 0, {}, undefined), v) - await v + const bf = e.fetchList?.[0] + t.equal(v, 1, 'stale value remains in valList while fetching') + t.type(bf, Promise, 'background fetch stored separately') + t.equal(e.valList[0], 1) + t.equal(e.backgroundFetch('key', 0, {}, undefined), bf) + await bf const v7 = await c.fetch('key', { allowStale: true, updateAgeOnGet: true, @@ -160,6 +163,17 @@ t.test('fetch without fetch method', async t => { t.matchSnapshot(status, 'status update') }) +t.test('fetch initializes ttl tracking from fetch options', async t => { + const c = new LRUCache({ + max: 2, + fetchMethod: async k => k, + }) + t.equal(c.getRemainingTTL(1), 0) + t.equal(await c.fetch(1, { ttl: 100 }), 1) + const ttl = c.getRemainingTTL(1) + t.ok(ttl > 0 && ttl <= 100) +}) + t.test('fetch options, signal', async t => { const statuses: LRUCache.Status[] = [] const s = (): LRUCache.Status => { @@ -603,8 +617,8 @@ t.test('verify inflight works as expected', async t => { c.fetch(1, { status: s() }), c.fetch(1), ] - t.match(e.valList, [Promise, null, null, null, null]) - t.equal(e.isBackgroundFetch(e.valList[0]), true, 'is background fetch') + t.equal(e.valList[0], undefined) + t.type(e.fetchList?.[0], Promise, 'is background fetch') t.equal(c.get(1, { status: s() }), undefined, 'get while fetching') const a = await Promise.all(promises) for (let i = 1; i < a.length; i++) { @@ -729,7 +743,8 @@ t.test('background update on timeout, return stale', async t => { const ac = new AbortController() const p = c.fetch(1, { signal: ac.signal }) await new Promise(res => queueMicrotask(res)) - t.match(e.valList[0], { __staleWhileFetching: 10 }) + t.equal(e.valList[0], 10) + t.type(e.fetchList?.[0], Promise) ac.abort(new Error('gimme the stale value')) t.equal(await p, 10) t.equal(c.get(1, { allowStale: true }), 10) @@ -743,15 +758,19 @@ t.test('background update on timeout, return stale', async t => { const ac2 = new AbortController() const p2 = c.fetch(1, { signal: ac2.signal }) await new Promise(res => queueMicrotask(res)) - t.match(e.valList[0], { __staleWhileFetching: 99 }) + t.equal(e.valList[0], 99) + t.type(e.fetchList?.[0], Promise) ac2.abort(new Error('gimme stale 99')) t.equal(await p2, 99) - t.match(e.valList[0], { __staleWhileFetching: 99 }) + t.equal(e.valList[0], 99) + t.type(e.fetchList?.[0], Promise) t.equal(c.get(1, { allowStale: true }), 99) - t.match(e.valList[0], { __staleWhileFetching: 99 }) + t.equal(e.valList[0], 99) + t.type(e.fetchList?.[0], Promise) clock.advance(200) await new Promise(res => queueMicrotask(res)) t.equal(e.valList[0], 99) + t.equal(e.fetchList?.[0], undefined) }) t.test('fetch context required if set in ctor type', async t => { @@ -800,9 +819,9 @@ t.test('has false for pending fetch without stale val', async t => { const p = c.fetch(1) const index = e.keyMap.get(1) as number t.not(index, undefined) - const bf = e.valList[index] as BackgroundFetch + const bf = e.fetchList?.[index] as BackgroundFetch t.type(bf, Promise, 'pending fetch') - t.equal(bf.hasOwnProperty('__staleWhileFetching'), true) + t.equal(e.valList[index], undefined) t.equal(c.has(1), false) clock.advance(10) const res = await p @@ -811,13 +830,13 @@ t.test('has false for pending fetch without stale val', async t => { } { - // background fetch that DOES have a __staleWhileFetching value + // background fetch that DOES have a stale while fetching value const p = c.fetch(1, { forceRefresh: true }) const index = e.keyMap.get(1) as number t.not(index, undefined) - const bf = e.valList[index] as BackgroundFetch + const bf = e.fetchList?.[index] as BackgroundFetch t.type(bf, Promise, 'pending fetch') - t.equal(bf.__staleWhileFetching, 1) + t.equal(e.valList[index], 1) t.equal(c.has(1), true) clock.advance(10) const res = await p From d6223bfed00b847b27bfc6779db6e4ddc99f291c Mon Sep 17 00:00:00 2001 From: Sean Doherty Date: Wed, 27 May 2026 17:54:27 -0500 Subject: [PATCH 2/2] Address background fetch review feedback --- src/index.ts | 107 +++++++++++++++++++++++------------------------ test/fetch.ts | 15 +++++++ test/onInsert.ts | 12 ++++++ 3 files changed, 79 insertions(+), 55 deletions(-) diff --git a/src/index.ts b/src/index.ts index d7a8b5e..3d6a016 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1273,7 +1273,7 @@ export class LRUCache { #valList: (V | undefined)[] #fetchList?: (BackgroundFetch | undefined)[] #abortControllerList?: (AbortController | undefined)[] - #returnedFetch?: NumberArray + #returnedFetch?: WeakSet> #next: NumberArray #prev: NumberArray #head: Index @@ -1779,21 +1779,13 @@ export class LRUCache { ac: AbortController, ) { if (!this.#fetchList) { - this.#fetchList = Array.from({ length: this.#max }).fill( - undefined, - ) as (BackgroundFetch | undefined)[] + this.#fetchList = Array.from({ length: this.#max }) } if (!this.#abortControllerList) { - this.#abortControllerList = Array.from({ length: this.#max }).fill( - undefined, - ) as (AbortController | undefined)[] - } - if (!this.#returnedFetch) { - this.#returnedFetch = this.#max ? new Uint8Array(this.#max) : [] + this.#abortControllerList = Array.from({ length: this.#max }) } this.#fetchList[index] = bf this.#abortControllerList[index] = ac - this.#returnedFetch[index] = 0 } #clearBackgroundFetch(index: Index) { @@ -1807,23 +1799,49 @@ export class LRUCache { #abortBackgroundFetch( index: Index, - reason: Error, + reason: () => Error, ): BackgroundFetch | undefined { const bf = this.#fetchList?.[index] if (bf) { - this.#abortControllerList?.[index]?.abort(reason) + this.#abortControllerList?.[index]?.abort(reason()) this.#clearBackgroundFetch(index) } return bf } #returnBackgroundFetch(index: Index, bf: BackgroundFetch) { - if (this.#returnedFetch) { - this.#returnedFetch[index] = 1 - } + this.#returnedFetch ??= new WeakSet() + this.#returnedFetch.add(bf) return bf } + #addItem( + k: K, + v: V | undefined, + size: LRUCache.Size, + status?: LRUCache.Status, + onInsert?: false, + ) { + const index = ( + this.#size === 0 ? this.#tail + : this.#free.length !== 0 ? this.#free.pop() + : this.#size === this.#max ? this.#evict(false) + : this.#size) as Index + this.#keyList[index] = k + this.#valList[index] = v + this.#keyMap.set(k, index) + this.#next[this.#tail] = index + this.#prev[index] = this.#tail + this.#tail = index + this.#size++ + this.#addItemSize(index, size, status) + if (status) status.set = 'add' + if (onInsert !== false && this.#hasOnInsert) { + this.#onInsert?.(v as V, k, 'add') + } + return index + } + *#indexes({ allowStale = this.allowStale } = {}) { if (this.#size) { for (let i = this.#tail; this.#isValidIndex(i); ) { @@ -2224,24 +2242,8 @@ export class LRUCache { let index = this.#size === 0 ? undefined : this.#keyMap.get(k) if (index === undefined) { // addition - index = ( - this.#size === 0 ? this.#tail - : this.#free.length !== 0 ? this.#free.pop() - : this.#size === this.#max ? this.#evict(false) - : this.#size) as Index - this.#keyList[index] = k - this.#valList[index] = v - this.#keyMap.set(k, index) - this.#next[this.#tail] = index - this.#prev[index] = this.#tail - this.#tail = index - this.#size++ - this.#addItemSize(index, size, status) - if (status) status.set = 'add' + index = this.#addItem(k, v, size, status) noUpdateTTL = false - if (this.#hasOnInsert) { - this.#onInsert?.(v as V, k, 'add') - } } else { // update this.#moveToTail(index) @@ -2279,7 +2281,13 @@ export class LRUCache { } if (this.#hasOnInsert) { - this.onInsert?.(v as V, k, v === oldVal ? 'update' : 'replace') + this.onInsert?.( + v as V, + k, + oldFetch && oldVal === undefined ? 'add' + : v === oldVal ? 'update' + : 'replace', + ) } } if (ttl !== 0 && !this.#ttls) { @@ -2308,12 +2316,9 @@ export class LRUCache { pop(): V | undefined { try { while (this.#size) { - const fetching = this.#fetchList?.[this.#head] !== undefined const val = this.#valList[this.#head] this.#evict(true) - if (fetching && val !== undefined) { - return val - } else if (!fetching && val !== undefined) { + if (val !== undefined) { return val } } @@ -2332,7 +2337,7 @@ export class LRUCache { const head = this.#head const k = this.#keyList[head] as K const v = this.#valList[head] as V | undefined - const bf = this.#abortBackgroundFetch(head, new Error('evicted')) + const bf = this.#abortBackgroundFetch(head, () => new Error('evicted')) if ( !bf && v !== undefined && @@ -2551,7 +2556,7 @@ export class LRUCache { const i = index as Index const staleWhileFetching = this.#fetchList?.[i] === bf ? this.#valList[i] : v - const returned = !!this.#returnedFetch?.[i] + const returned = !!this.#returnedFetch?.has(bf) if (this.#fetchList?.[i] === bf) { // if we allow stale on fetch rejections, then we need to ensure that // the stale value is not removed from the cache when the fetch fails. @@ -2604,19 +2609,7 @@ export class LRUCache { if (index === undefined) { const { ttl = this.ttl } = fetchOpts.options - index = ( - this.#size === 0 ? this.#tail - : this.#free.length !== 0 ? this.#free.pop() - : this.#size === this.#max ? this.#evict(false) - : this.#size) as Index - this.#keyList[index] = k - this.#valList[index] = undefined - this.#keyMap.set(k, index) - this.#next[this.#tail] = index - this.#prev[index] = this.#tail - this.#tail = index - this.#size++ - this.#addItemSize(index, 0) + index = this.#addItem(k, undefined, 0, undefined, false) if (ttl !== 0 && !this.#ttls) { this.#initializeTTLTracking() } @@ -3117,7 +3110,7 @@ export class LRUCache { const v = this.#valList[index] const bf = this.#abortBackgroundFetch( index, - new Error('deleted'), + () => new Error('deleted'), ) if ( !bf && @@ -3168,7 +3161,10 @@ export class LRUCache { #clear(reason: LRUCache.DisposeReason) { for (const index of this.#rindexes({ allowStale: true })) { const v = this.#valList[index] - const bf = this.#abortBackgroundFetch(index, new Error('deleted')) + const bf = this.#abortBackgroundFetch( + index, + () => new Error('deleted'), + ) if (!bf && v !== undefined) { const k = this.#keyList[index] if (this.#hasDispose) { @@ -3184,6 +3180,7 @@ export class LRUCache { this.#valList.fill(undefined) this.#fetchList?.fill(undefined) this.#abortControllerList?.fill(undefined) + this.#returnedFetch = undefined this.#keyList.fill(undefined) if (this.#ttls && this.#starts) { this.#ttls.fill(0) diff --git a/test/fetch.ts b/test/fetch.ts index ab6fd5f..3bad66c 100644 --- a/test/fetch.ts +++ b/test/fetch.ts @@ -508,6 +508,21 @@ t.test('allowStaleOnFetchRejection', async t => { t.equal(c.get(1), undefined) }) +t.test('returned background fetch rejects after slot reuse', async t => { + const resolves: Record void> = {} + const c = new LRU({ + max: 1, + fetchMethod: k => new Promise(resolve => (resolves[k] = resolve)), + }) + + const p1 = c.fetch('one') + const p2 = c.fetch('two') + + await t.rejects(p1, { message: 'evicted' }) + resolves.two?.(2) + t.equal(await p2, 2) +}) + t.test('placeholder promise is not removed when resolving', async t => { const resolves: Record void> = {} const c = new LRU({ diff --git a/test/onInsert.ts b/test/onInsert.ts index 9b1ad7f..e5d5bec 100644 --- a/test/onInsert.ts +++ b/test/onInsert.ts @@ -74,3 +74,15 @@ t.test('onInsert with update (same value)', t => { t.end() }) + +t.test('onInsert with fetch-created entry', async t => { + const inserted: any[] = [] + const c = new LRU({ + max: 5, + onInsert: (v, k, r) => inserted.push([v, k, r]), + fetchMethod: async k => `value:${k}`, + }) + + t.equal(await c.fetch('key'), 'value:key') + t.strictSame(inserted, [['value:key', 'key', 'add']]) +})