fix: cloneData for concurrent requests (#921)

* code

* code

* reuse my biome config

* fix breaking change on 1.6.0

* lint

biome.json

@@ -1,31 +1,6 @@
 {
   "$schema": "https://biomejs.dev/schemas/1.9.0/schema.json",
-  "organizeImports": {
-    "enabled": true
-  },
-  "linter": {
-    "enabled": true,
-    "rules": {
-      "recommended": true,
-      "style": {
-        "noNonNullAssertion": "off",
-        "noParameterAssign": "off"
-      },
-      "suspicious": {
-        "noExplicitAny": "off"
-      }
-    }
-  },
-  "formatter": {
-    "lineWidth": 100,
-    "indentStyle": "space"
-  },
-  "javascript": {
-    "formatter": {
-      "quoteStyle": "single",
-      "trailingCommas": "none"
-    }
-  },
+  "extends": ["@arthurfiorette/biomejs-config"],
   "files": {
     "ignore": [
       "build/**/*",

docs/src/config.md

@@ -62,14 +62,14 @@ In any persistent cache scenario where hitting over 77K unique keys is a possibi
 
 <Badge text="optional" type="warning"/>
 
-- Type: `Record<string, Deferred<CachedResponse>>`
-- Default: `{}`
+- Type: `Map<string, Deferred<void>>`
+- Default: `new Map`
 
 A simple object that will hold a promise for each pending request. Used to handle
 concurrent requests.
 
-You'd normally not need to change this, but it is exposed in case you need to use it as
-some sort of listener of know when a request is waiting for other to finish.
+You shouldn't change this property, but it is exposed in case you need to use it as some
+sort of listener or know when a request is waiting for others to finish.
 
 ## headerInterpreter
 
@@ -102,7 +102,10 @@ The possible returns are:
 ::: details Example of a custom headerInterpreter
 
 ```ts
-import { setupCache, type HeaderInterpreter } from 'axios-cache-interceptor';
+import {
+  setupCache,
+  type HeaderInterpreter
+} from 'axios-cache-interceptor';
 
 const myHeaderInterpreter: HeaderInterpreter = (headers) => {
   if (headers['x-my-custom-header']) {
@@ -186,7 +189,8 @@ setupCache(axiosInstance, { debug: console.log });
 
 // Own logging platform.
 setupCache(axiosInstance, {
-  debug: ({ id, msg, data }) => myLoggerExample.emit({ id, msg, data })
+  debug: ({ id, msg, data }) =>
+    myLoggerExample.emit({ id, msg, data })
 });
 
 // Disables debug. (default)

docs/src/config/request-specifics.md

@@ -148,7 +148,7 @@ and in this [StackOverflow](https://stackoverflow.com/a/62781874/14681561) answe
 <Badge text="optional" type="warning"/>
 
 - Type: `Method[]`
-- Default: `["get"]`
+- Default: `["get", "head"]`
 
 Specifies which methods we should handle and cache. This is where you can enable caching
 to `POST`, `PUT`, `DELETE` and other methods, as the default is only `GET`.

docs/src/guide/storages.md

@@ -38,7 +38,10 @@ For long running processes, you can avoid memory leaks by using playing with the
 
 ```ts
 import Axios from 'axios';
-import { setupCache, buildMemoryStorage } from 'axios-cache-interceptor';
+import {
+  setupCache,
+  buildMemoryStorage
+} from 'axios-cache-interceptor';
 
 setupCache(axios, {
   // You don't need to to that, as it is the default option.
@@ -140,7 +143,8 @@ simple object to build the storage. It has 3 methods:
   storage or `undefined` if not found.
 
 - `clear() => MaybePromise<void>`:
-  Clears all data from storage.
+  Clears all data from storage. **This method isn't used by the interceptor itself**, instead, its
+  here for you to use it programmatically.
 
 ## Third Party Storages
 
@@ -240,7 +244,7 @@ const indexedDbStorage = buildStorage({
 
 ### Node Cache
 
-This example implementation uses [node-cache](https://github.com/node-cache/node-cache) as a storage method. Do note 
+This example implementation uses [node-cache](https://github.com/node-cache/node-cache) as a storage method. Do note
 that this library is somewhat old, however it appears to work at the time of writing.
 
 ```ts

package.json

@@ -1,6 +1,6 @@
 {
   "name": "axios-cache-interceptor",
-  "version": "1.6.0",
+  "version": "1.6.1",
   "description": "Cache interceptor for axios",
   "keywords": ["axios", "cache", "interceptor", "adapter", "http", "plugin", "wrapper"],
   "homepage": "https://axios-cache-interceptor.js.org",
@@ -11,7 +11,6 @@
   "author": "Arthur Fiorette <npm@arthur.place>",
   "sideEffects": false,
   "type": "module",
-  "source": "./src/index.ts",
   "exports": {
     ".": {
       "require": "./dist/index.cjs",
@@ -28,6 +27,7 @@
   "jsdelivr": "./dist/index.bundle.js",
   "unpkg": "./dist/index.bundle.js",
   "module": "./dist/index.mjs",
+  "source": "./src/index.ts",
   "types": "./dist/index.d.ts",
   "scripts": {
     "benchmark": "cd benchmark && pnpm start",
@@ -35,13 +35,13 @@
     "docs:build": "vitepress build docs",
     "docs:dev": "vitepress dev docs --port 1227",
     "docs:serve": "vitepress serve docs",
-    "test": "c8 --reporter lcov --reporter text node --import ./test/setup.js --enable-source-maps --test test/**/*.test.ts",
-    "test:only": "c8 --reporter lcov --reporter text node --import ./test/setup.js --enable-source-maps --test-only",
-    "version": "auto-changelog -p && cp CHANGELOG.md docs/src/others/changelog.md && git add CHANGELOG.md docs/src/others/changelog.md",
     "format": "biome format --write .",
     "lint": "biome check .",
+    "lint:ci": "biome ci .",
     "lint:fix": "biome check --write --unsafe .",
-    "lint:ci": "biome ci ."
+    "test": "c8 --reporter lcov --reporter text node --import ./test/setup.js --enable-source-maps --test test/**/*.test.ts",
+    "test:only": "c8 --reporter lcov --reporter text node --import ./test/setup.js --enable-source-maps --test-only",
+    "version": "auto-changelog -p && cp CHANGELOG.md docs/src/others/changelog.md && git add CHANGELOG.md docs/src/others/changelog.md"
   },
   "resolutions": {
     "colors": "1.4.0"
@@ -52,6 +52,7 @@
     "object-code": "1.3.3"
   },
   "devDependencies": {
+    "@arthurfiorette/biomejs-config": "1.0.5",
     "@biomejs/biome": "1.9.4",
     "@swc-node/register": "1.9.0",
     "@swc/helpers": "0.5.13",

pnpm-lock.yaml

@@ -21,6 +21,9 @@ importers:
         specifier: 1.3.3
         version: 1.3.3
     devDependencies:
+      '@arthurfiorette/biomejs-config':
+        specifier: 1.0.5
+        version: 1.0.5
       '@biomejs/biome':
         specifier: 1.9.4
         version: 1.9.4
@@ -129,6 +132,9 @@ packages:
     resolution: {integrity: sha512-lFMjJTrFL3j7L9yBxwYfCq2k6qqwHyzuUl/XBnif78PWTJYyL/dfowQHWE3sp6U6ZzqWiiIZnpTMO96zhkjwtg==}
     engines: {node: '>=6.0.0'}
 
+  '@arthurfiorette/biomejs-config@1.0.5':
+    resolution: {integrity: sha512-2+r6+zsme3IuIr3Vuba7oZyif748EYnFOPGm8kahY4AM8q9EzJZRWNfaAveuV0+PTrLaIhZPhUKIkAhg5JLQ5A==}
+
   '@babel/code-frame@7.23.5':
     resolution: {integrity: sha512-CgH3s1a96LipHCmSUmYFPwY7MNx8C3avkq7i4Wl3cfa662ldtUe4VM1TPXX70pfmrlWTb6jLqTYrZyT2ZTJBgA==}
     engines: {node: '>=6.9.0'}
@@ -3441,6 +3447,8 @@ snapshots:
       '@jridgewell/gen-mapping': 0.3.3
       '@jridgewell/trace-mapping': 0.3.18
 
+  '@arthurfiorette/biomejs-config@1.0.5': {}
+
   '@babel/code-frame@7.23.5':
     dependencies:
       '@babel/highlight': 7.23.4

src/cache/cache.ts

@@ -4,7 +4,6 @@ import type { HeaderInterpreter } from '../header/types.js';
 import type { AxiosInterceptor } from '../interceptors/build.js';
 import type {
   AxiosStorage,
-  CachedResponse,
   CachedStorageValue,
   LoadingStorageValue,
   StaleStorageValue
@@ -86,7 +85,7 @@ export interface CacheProperties<R = unknown, D = unknown> {
    * We use `methods` in a per-request configuration setup because sometimes you have
    * exceptions to the method rule.
    *
-   * @default ['get']
+   * @default ['get', 'head']
    * @see https://axios-cache-interceptor.js.org/config/request-specifics#cache-methods
    */
   methods: Lowercase<Method>[];
@@ -261,10 +260,10 @@ export interface CacheInstance {
    * You'd normally not need to change this, but it is exposed in case you need to use it
    * as some sort of listener of know when a request is waiting for other to finish.
    *
-   * @default { }
+   * @default new Map()
    * @see https://axios-cache-interceptor.js.org/config#waiting
    */
-  waiting: Record<string, Deferred<CachedResponse>>;
+  waiting: Map<string, Deferred<void>>;
 
   /**
    * The function used to interpret all headers from a request and determine a time to

src/cache/create.ts

@@ -39,7 +39,7 @@ export function setupCache(axios: AxiosInstance, options: CacheOptions = {}): Ax
     throw new Error('Use buildStorage() function');
   }
 
-  axiosCache.waiting = options.waiting || {};
+  axiosCache.waiting = options.waiting || new Map();
 
   axiosCache.generateKey = options.generateKey || defaultKeyGenerator;
 

src/interceptors/request.ts

@@ -94,7 +94,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
       // This checks for simultaneous access to a new key. The js event loop jumps on the
       // first await statement, so the second (asynchronous call) request may have already
       // started executing.
-      if (axios.waiting[config.id] && !overrideCache) {
+      if (axios.waiting.has(config.id) && !overrideCache) {
         cache = (await axios.storage.get(config.id, config)) as
           | CachedStorageValue
           | LoadingStorageValue;
@@ -116,11 +116,12 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
       }
 
       // Create a deferred to resolve other requests for the same key when it's completed
-      axios.waiting[config.id] = deferred();
+      const def = deferred<void>();
+      axios.waiting.set(config.id, def);
 
       // Adds a default reject handler to catch when the request gets aborted without
       // others waiting for it.
-      axios.waiting[config.id]!.catch(() => undefined);
+      def.catch(() => undefined);
 
       await axios.storage.set(
         config.id,
@@ -178,7 +179,7 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
     let cachedResponse: CachedResponse;
 
     if (cache.state === 'loading') {
-      const deferred = axios.waiting[config.id];
+      const deferred = axios.waiting.get(config.id);
 
       // The deferred may not exists when the process is using a persistent
       // storage and cancelled  in the middle of a request, this would result in
@@ -200,7 +201,28 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
       }
 
       try {
-        cachedResponse = await deferred;
+        // Deferred can't reuse the value because the user's storage might clone
+        // or mutate the value, so we need to ask it again.
+        // For example with memoryStorage + cloneData
+        await deferred;
+        const state = await axios.storage.get(config.id, config);
+
+        // This is a cache mismatch and should never happen, but in case it does,
+        // we need to redo the request all over again.
+        /* c8 ignore start */
+        if (!state.data) {
+          if (__ACI_DEV__) {
+            axios.debug({
+              id: config.id,
+              msg: 'Deferred resolved, but no data was found, requesting again'
+            });
+          }
+
+          return onFulfilled(config);
+        }
+        /* c8 ignore end */
+
+        cachedResponse = state.data;
       } catch (err) {
         if (__ACI_DEV__) {
           axios.debug({
@@ -211,10 +233,11 @@ export function defaultRequestInterceptor(axios: AxiosCacheInstance) {
         }
 
         // Hydrates any UI temporarily, if cache is available
-        /* c8 ignore next 3 */
+        /* c8 ignore start */
         if (cache.data) {
           await config.cache.hydrate?.(cache);
         }
+        /* c8 ignore end */
 
         // The deferred is rejected when the request that we are waiting rejects its cache.
         // In this case, we need to redo the request all over again.

src/interceptors/response.ts

@@ -20,9 +20,12 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI
     await axios.storage.remove(responseId, config);
 
     // Rejects the deferred, if present
-    axios.waiting[responseId]?.reject();
+    const deferred = axios.waiting.get(responseId);
 
-    delete axios.waiting[responseId];
+    if (deferred) {
+      deferred.reject();
+      axios.waiting.delete(responseId);
+    }
   };
 
   const onFulfilled: ResponseInterceptor['onFulfilled'] = async (response) => {
@@ -200,12 +203,15 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI
       data
     };
 
+    // Define this key as cache on the storage
+    await axios.storage.set(response.id, newCache, config);
+
     // Resolve all other requests waiting for this response
-    const waiting = axios.waiting[response.id];
+    const waiting = axios.waiting.get(response.id);
 
     if (waiting) {
-      waiting.resolve(newCache.data);
-      delete axios.waiting[response.id];
+      waiting.resolve();
+      axios.waiting.delete(response.id);
 
       if (__ACI_DEV__) {
         axios.debug({
@@ -215,9 +221,6 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI
       }
     }
 
-    // Define this key as cache on the storage
-    await axios.storage.set(response.id, newCache, config);
-
     if (__ACI_DEV__) {
       axios.debug({
         id: response.id,
@@ -323,10 +326,6 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI
         // staleIfError is the number of seconds that stale is allowed to be used
         (typeof staleIfError === 'number' && cache.createdAt + staleIfError > Date.now())
       ) {
-        // Resolve all other requests waiting for this response
-        axios.waiting[id]?.resolve(cache.data);
-        delete axios.waiting[id];
-
         // re-mark the cache as stale
         await axios.storage.set(
           id,
@@ -337,6 +336,20 @@ export function defaultResponseInterceptor(axios: AxiosCacheInstance): ResponseI
           },
           config
         );
+        // Resolve all other requests waiting for this response
+        const waiting = axios.waiting.get(id);
+
+        if (waiting) {
+          waiting.resolve();
+          axios.waiting.delete(id);
+
+          if (__ACI_DEV__) {
+            axios.debug({
+              id,
+              msg: 'Found waiting deferred(s) and resolved them'
+            });
+          }
+        }
 
         if (__ACI_DEV__) {
           axios.debug({

src/storage/build.ts

@@ -65,13 +65,6 @@ export interface BuildStorage extends Omit<AxiosStorage, 'get'> {
     key: string,
     currentRequest?: CacheRequestConfig
   ) => MaybePromise<StorageValue | undefined>;
-
-  /**
-   * Deletes all values from the storage.
-   *
-   * @see https://axios-cache-interceptor.js.org/guide/storages#buildstorage
-   */
-  clear: () => MaybePromise<void>;
 }
 
 /**

src/storage/memory.ts

@@ -1,12 +1,17 @@
 import { buildStorage, canStale, isExpired } from './build.js';
-import type { AxiosStorage, NotEmptyStorageValue, StorageValue } from './types.js';
+import type { AxiosStorage, StorageValue } from './types.js';
 
+/* c8 ignore start */
 /**
- * Modern function to natively deep clone.
- *
- * @link https://caniuse.com/mdn-api_structuredclone (07/03/2022 -> 59.4%)
+ * Clones an object using the structured clone algorithm if available, otherwise
+ * it uses JSON.parse(JSON.stringify(value)).
  */
-declare const structuredClone: (<T>(value: T) => T) | undefined;
+const clone: <T>(value: T) => T =
+  // https://caniuse.com/mdn-api_structuredclone (10/18/2023 92.51%)
+  typeof structuredClone === 'function'
+    ? structuredClone
+    : (value) => JSON.parse(JSON.stringify(value));
+/* c8 ignore stop */
 
 /**
  * Creates a simple in-memory storage. This means that if you need to persist data between
@@ -69,15 +74,9 @@ export function buildMemoryStorage(
         }
       }
 
-      storage.data[key] =
-        // Clone the value before storing to prevent future mutations
-        // from affecting cached data.
-        cloneData === 'double'
-          ? /* c8 ignore next 3 */
-            typeof structuredClone === 'function'
-            ? structuredClone(value)
-            : (JSON.parse(JSON.stringify(value)) as NotEmptyStorageValue)
-          : value;
+      // Clone the value before storing to prevent future mutations
+      // from affecting cached data.
+      storage.data[key] = cloneData === 'double' ? clone(value) : value;
     },
 
     remove: (key) => {
@@ -87,16 +86,7 @@ export function buildMemoryStorage(
     find: (key) => {
       const value = storage.data[key];
 
-      /* c8 ignore next 7 */
-      if (cloneData && value !== undefined) {
-        if (typeof structuredClone === 'function') {
-          return structuredClone(value);
-        }
-
-        return JSON.parse(JSON.stringify(value)) as StorageValue;
-      }
-
-      return value;
+      return cloneData && value !== undefined ? clone(value) : value;
     },
 
     clear: () => {
@@ -123,8 +113,6 @@ export function buildMemoryStorage(
       value = storage.data[key]!;
 
       if (value.state === 'empty') {
-        // this storage returns void.
-
         storage.remove(key);
         continue;
       }

src/storage/types.ts

@@ -136,9 +136,12 @@ export interface AxiosStorage {
   get: (key: string, currentRequest?: CacheRequestConfig) => MaybePromise<StorageValue>;
 
   /**
-   * Deletes all values from the storage.
+   * Deletes all values from the storage, this method isn't used by the interceptor
+   * and is here just for convenience.
+   *
+   * **All native storages implement them, but it's not required.**
    *
    * @see https://axios-cache-interceptor.js.org/guide/storages#buildstorage
    */
-  clear: () => MaybePromise<void>;
+  clear?: () => MaybePromise<void>;
 }

test/interceptors/request.test.ts

@@ -4,6 +4,7 @@ import { setTimeout } from 'node:timers/promises';
 import type { AxiosAdapter, AxiosResponse } from 'axios';
 import type { CacheRequestConfig, InternalCacheRequestConfig } from '../../src/cache/axios.js';
 import { Header } from '../../src/header/headers.js';
+import { buildMemoryStorage } from '../../src/index.js';
 import type { LoadingStorageValue } from '../../src/storage/types.js';
 import { mockAxios } from '../mocks/axios.js';
 import { mockDateNow } from '../utils.js';
@@ -227,7 +228,7 @@ describe('Request Interceptor', () => {
     // it still has a waiting entry.
     const { state } = await axios.storage.get(ID);
     assert.equal(state, 'empty');
-    assert.ok(axios.waiting[ID]);
+    assert.ok(axios.waiting.get(ID));
 
     // This line should throw an error if this bug isn't fixed.
     await axios.get('url', { id: ID });
@@ -235,7 +236,7 @@ describe('Request Interceptor', () => {
     const { state: newState } = await axios.storage.get(ID);
 
     assert.notEqual(newState, 'empty');
-    assert.equal(axios.waiting[ID], undefined);
+    assert.equal(axios.waiting.get(ID), undefined);
   });
 
   it('`cache.override = true` with previous cache', async () => {
@@ -451,4 +452,40 @@ describe('Request Interceptor', () => {
     assert.equal(req5.cached, false);
     assert.equal(req5.stale, undefined);
   });
+
+  it('clone works with concurrent requests', async () => {
+    const axios = mockAxios(
+      {
+        storage: buildMemoryStorage('double')
+      },
+      undefined,
+      undefined,
+      () => ({ a: 1 })
+    );
+
+    await Promise.all(
+      Array.from({ length: 10 }, async () => {
+        const result = await axios.get<{ a: 1 }>('/url');
+        result.data.a++;
+        assert.equal(result.data.a, 2);
+      })
+    );
+  });
+
+  it('clone works with sequential requests', async () => {
+    const axios = mockAxios(
+      {
+        storage: buildMemoryStorage('double')
+      },
+      undefined,
+      undefined,
+      () => ({ a: 1 })
+    );
+
+    for (let i = 0; i < 10; i++) {
+      const result = await axios.get<{ a: 1 }>('/url');
+      result.data.a++;
+      assert.equal(result.data.a, 2);
+    }
+  });
 });

test/mocks/axios.ts

@@ -9,7 +9,8 @@ export const XMockRandom = 'x-mock-random';
 export function mockAxios(
   options: CacheOptions = {},
   responseHeaders: Record<string, string> = {},
-  instance = Axios.create()
+  instance = Axios.create(),
+  data: () => any = () => true
 ): AxiosCacheInstance {
   const axios = setupCache(instance, options);
 
@@ -30,7 +31,7 @@ export function mockAxios(
         config,
         { config },
         {
-          data: true,
+          data: data(),
           status,
           statusText,
           headers: {
@@ -45,7 +46,7 @@ export function mockAxios(
     }
 
     return {
-      data: true,
+      data: data(),
       status,
       statusText,
       headers: {