From fcb797eb456f55077bd3baa265d45f415a667d84 Mon Sep 17 00:00:00 2001
From: isaacs <i@izs.me>
Date: Thu, 10 Aug 2023 14:29:21 -0700
Subject: [PATCH] Call dispose() when fetch overwrites a value

This corrects an oversight in the disposal flow on set overwrites.

We don't call dispose when creating the BackgroundFetch, because of
course, the thing hasn't been replaced yet.

And we don't call dispose when _replacing_ a BackgroundFetch, because
that isn't a value, it's just a potential value, so we abort the fetch
and forget about it (which is a no-op if the set() in question is the
resolution of that BackgroundFetch).

The missing piece is that we *do* have to dispose() the
`__staleWhileFetching` value on that BackgroundFetch promise, if there
is one, when overwriting a BackgroundFetch.

Fix: #309
---
 src/index.ts  |  9 +++++++++
 test/fetch.ts | 26 ++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/index.ts b/src/index.ts
index eeb4ed8..400f4cc 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1682,6 +1682,15 @@ export class LRUCache<K extends {}, V extends {}, FC = unknown> {
       if (v !== oldVal) {
         if (this.#hasFetchMethod && this.#isBackgroundFetch(oldVal)) {
           oldVal.__abortController.abort(new Error('replaced'))
+          const { __staleWhileFetching: s } = oldVal
+          if (s !== undefined && !noDisposeOnSet) {
+            if (this.#hasDispose) {
+              this.#dispose?.(s as V, k, 'set')
+            }
+            if (this.#hasDisposeAfter) {
+              this.#disposed?.push([s as V, k, 'set'])
+            }
+          }
         } else if (!noDisposeOnSet) {
           if (this.#hasDispose) {
             this.#dispose?.(oldVal as V, k, 'set')
diff --git a/test/fetch.ts b/test/fetch.ts
index b0fa33e..984e78d 100644
--- a/test/fetch.ts
+++ b/test/fetch.ts
@@ -2,7 +2,7 @@ if (typeof performance === 'undefined') {
   global.performance = require('perf_hooks').performance
 }
 import t from 'tap'
-import { LRUCache, BackgroundFetch } from '../'
+import { BackgroundFetch, LRUCache } from '../'
 import { expose } from './fixtures/expose'
 
 const fn: LRUCache.Fetcher<any, any> = async (_, v) =>
@@ -720,7 +720,10 @@ t.test('allowStaleOnFetchAbort', async t => {
   const p = c.fetch(1, { signal: ac.signal })
   ac.abort(new Error('gimme the stale value'))
   t.equal(await p, 10)
-  t.equal(c.get(1, { allowStale: true, noDeleteOnStaleGet: true }), 10)
+  t.equal(
+    c.get(1, { allowStale: true, noDeleteOnStaleGet: true }),
+    10
+  )
   const p2 = c.fetch(1)
   c.set(1, 100)
   t.equal(await p2, 10)
@@ -844,3 +847,22 @@ t.test('has false for pending fetch without stale val', async t => {
     t.equal(c.has(1), true)
   }
 })
+
+t.test('properly dispose when using fetch', async t => {
+  const disposes: [number, number, string][] = []
+  const disposeAfters: [number, number, string][] = []
+  let i = 0
+  const c = new LRUCache<number, number>({
+    max: 3,
+    ttl: 10,
+    dispose: (key, val, reason) => disposes.push([key, val, reason]),
+    disposeAfter: (key, val, reason) =>
+      disposeAfters.push([key, val, reason]),
+    fetchMethod: async () => Promise.resolve(i++),
+  })
+  t.equal(await c.fetch(1), 0)
+  clock.advance(20)
+  t.equal(await c.fetch(1), 1)
+  t.strictSame(disposes, [[0, 1, 'set']])
+  t.strictSame(disposeAfters, [[0, 1, 'set']])
+})