enhance: Change NetworkManager bookkeeping data structure for inflight fetches

.changeset/sour-horses-give.md

@@ -0,0 +1,22 @@
+---
+'@data-client/core': minor
+'@data-client/test': minor
+---
+
+Change NetworkManager bookkeeping data structure for inflight fetches
+
+BREAKING CHANGE: NetworkManager.fetched, NetworkManager.rejectors, NetworkManager.resolvers, NetworkManager.fetchedAt
+  -> NetworkManager.fetching
+
+
+#### Before
+
+```ts
+if (action.key in this.fetched)
+```
+
+#### After
+
+```ts
+if (this.fetching.has(action.key))
+```

packages/core/src/index.ts

@@ -26,6 +26,7 @@ export {
   default as NetworkManager,
   ResetError,
 } from './manager/NetworkManager.js';
+export type { FetchingMeta } from './manager/NetworkManager.js';
 export * from './state/GCPolicy.js';
 export {
   default as createReducer,

packages/core/src/manager/NetworkManager.ts

@@ -5,7 +5,6 @@ import type {
   FetchAction,
   Manager,
   ActionTypes,
-  MiddlewareAPI,
   Middleware,
   SetResponseAction,
 } from '../types.js';
@@ -18,6 +17,13 @@ export class ResetError extends Error {
   }
 }
 
+export interface FetchingMeta {
+  promise: Promise<any>;
+  resolve: (value?: any) => void;
+  reject: (value?: any) => void;
+  fetchedAt: number;
+}
+
 /** Handles all async network dispatches
  *
  * Dedupes concurrent requests by keeping track of all fetches in flight
@@ -28,10 +34,7 @@ export class ResetError extends Error {
  * @see https://dataclient.io/docs/api/NetworkManager
  */
 export default class NetworkManager implements Manager {
-  protected fetched: { [k: string]: Promise<any> } = Object.create(null);
-  protected resolvers: { [k: string]: (value?: any) => void } = {};
-  protected rejectors: { [k: string]: (value?: any) => void } = {};
-  protected fetchedAt: { [k: string]: number } = {};
+  protected fetching: Map<string, FetchingMeta> = new Map();
   declare readonly dataExpiryLength: number;
   declare readonly errorExpiryLength: number;
   protected controller: Controller = new Controller();
@@ -61,7 +64,7 @@ export default class NetworkManager implements Manager {
         case SET_RESPONSE:
           // only set after new state is computed
           return next(action).then(() => {
-            if (action.key in this.fetched) {
+            if (this.fetching.has(action.key)) {
               // Note: meta *must* be set by reducer so this should be safe
               const error = controller.getState().meta[action.key]?.error;
               // processing errors result in state meta having error, so we should reject the promise
@@ -80,14 +83,16 @@ export default class NetworkManager implements Manager {
             }
           });
         case RESET: {
-          const rejectors = { ...this.rejectors };
+          // take snapshot of rejectors at this point in time
+          // we must use Array.from since iteration does not freeze state at this point in time
+          const fetches = Array.from(this.fetching.values());
 
           this.clearAll();
           return next(action).then(() => {
             // there could be external listeners to the promise
             // this must happen after commit so our own rejector knows not to dispatch an error based on this
-            for (const k in rejectors) {
-              rejectors[k](new ResetError());
+            for (const { reject } of fetches) {
+              reject(new ResetError());
             }
           });
         }
@@ -112,28 +117,29 @@ export default class NetworkManager implements Manager {
   /** Used by DevtoolsManager to determine whether to log an action */
   skipLogging(action: ActionTypes) {
     /* istanbul ignore next */
-    return action.type === FETCH && action.key in this.fetched;
+    return action.type === FETCH && this.fetching.has(action.key);
   }
 
   allSettled() {
-    const fetches = Object.values(this.fetched);
-    if (fetches.length) return Promise.allSettled(fetches);
+    if (this.fetching.size)
+      return Promise.allSettled(
+        this.fetching.values().map(({ promise }) => promise),
+      );
   }
 
   /** Clear all promise state */
   protected clearAll() {
-    for (const k in this.rejectors) {
+    for (const k of this.fetching.keys()) {
       this.clear(k);
     }
   }
 
   /** Clear promise state for a given key */
   protected clear(key: string) {
-    this.fetched[key].catch(() => {});
-    delete this.resolvers[key];
-    delete this.rejectors[key];
-    delete this.fetched[key];
-    delete this.fetchedAt[key];
+    if (this.fetching.has(key)) {
+      (this.fetching.get(key) as FetchingMeta).promise.catch(() => {});
+      this.fetching.delete(key);
+    }
   }
 
   protected getLastReset() {
@@ -226,14 +232,14 @@ export default class NetworkManager implements Manager {
    */
   protected handleSet(action: SetResponseAction) {
     // this can still turn out to be untrue since this is async
-    if (action.key in this.fetched) {
-      let promiseHandler: (value?: any) => void;
+    if (this.fetching.has(action.key)) {
+      const { reject, resolve } = this.fetching.get(action.key) as FetchingMeta;
       if (action.error) {
-        promiseHandler = this.rejectors[action.key];
+        reject(action.response);
       } else {
-        promiseHandler = this.resolvers[action.key];
+        resolve(action.response);
       }
-      promiseHandler(action.response);
+
       // since we're resolved we no longer need to keep track of this promise
       this.clear(action.key);
     }
@@ -253,19 +259,18 @@ export default class NetworkManager implements Manager {
     key: string,
     fetch: () => Promise<any>,
     fetchedAt: number,
-  ) {
+  ): Promise<any> {
     const lastReset = this.getLastReset();
+    let fetchMeta = this.fetching.get(key);
+
     // we're already fetching so reuse the promise
     // fetches after reset do not count
-    if (key in this.fetched && this.fetchedAt[key] > lastReset) {
-      return this.fetched[key];
+    if (fetchMeta && fetchMeta.fetchedAt > lastReset) {
+      return fetchMeta.promise;
     }
 
-    this.fetched[key] = new Promise((resolve, reject) => {
-      this.resolvers[key] = resolve;
-      this.rejectors[key] = reject;
-    });
-    this.fetchedAt[key] = fetchedAt;
+    fetchMeta = newFetchMeta(fetchedAt);
+    this.fetching.set(key, fetchMeta);
 
     this.idleCallback(
       () => {
@@ -277,7 +282,7 @@ export default class NetworkManager implements Manager {
       { timeout: 500 },
     );
 
-    return this.fetched[key];
+    return fetchMeta.promise;
   }
 
   /** Calls the callback when client is not 'busy' with high priority interaction tasks
@@ -291,3 +296,12 @@ export default class NetworkManager implements Manager {
     callback();
   }
 }
+
+function newFetchMeta(fetchedAt: number): FetchingMeta {
+  const fetchMeta = { fetchedAt } as FetchingMeta;
+  fetchMeta.promise = new Promise((resolve, reject) => {
+    fetchMeta.resolve = resolve;
+    fetchMeta.reject = reject;
+  });
+  return fetchMeta;
+}

packages/test/src/makeRenderDataClient/index.tsx

@@ -83,9 +83,7 @@ export default function makeRenderDataHook(
     // TODO: move to return value
     renderDataClient.cleanup = () => {
       nm.cleanupDate = Infinity;
-      Object.values(nm['rejectors'] as Record<string, any>).forEach(rej => {
-        rej();
-      });
+      nm['fetching'].forEach(({ reject }) => reject());
       nm['clearAll']();
       managers.forEach(manager => manager.cleanup());
     };

website/src/components/Playground/editor-types/@data-client/core.d.ts

@@ -889,6 +889,12 @@ declare class ResetError extends Error {
     name: string;
     constructor();
 }
+interface FetchingMeta {
+    promise: Promise<any>;
+    resolve: (value?: any) => void;
+    reject: (value?: any) => void;
+    fetchedAt: number;
+}
 /** Handles all async network dispatches
  *
  * Dedupes concurrent requests by keeping track of all fetches in flight
@@ -899,18 +905,7 @@ declare class ResetError extends Error {
  * @see https://dataclient.io/docs/api/NetworkManager
  */
 declare class NetworkManager implements Manager {
-    protected fetched: {
-        [k: string]: Promise<any>;
-    };
-    protected resolvers: {
-        [k: string]: (value?: any) => void;
-    };
-    protected rejectors: {
-        [k: string]: (value?: any) => void;
-    };
-    protected fetchedAt: {
-        [k: string]: number;
-    };
+    protected fetching: Map<string, FetchingMeta>;
     readonly dataExpiryLength: number;
     readonly errorExpiryLength: number;
     protected controller: Controller;
@@ -1371,4 +1366,4 @@ interface Props {
     shouldLogout?: (error: UnknownError) => boolean;
 }
 
-export { type AbstractInstanceType, type ActionMeta, type ActionTypes, type ConnectionListener, Controller, type CreateCountRef, type DataClientDispatch, DefaultConnectionListener, type Denormalize, type DenormalizeNullable, type DevToolsConfig, DevToolsManager, type Dispatch, type EndpointExtraOptions, type EndpointInterface, type EndpointUpdateFunction, type EntityInterface, type ErrorTypes, type ExpireAllAction, ExpiryStatus, type FetchAction, type FetchFunction, type FetchMeta, type GCAction, type GCInterface, type GCOptions, GCPolicy, type GenericDispatch, type INormalizeDelegate, type IQueryDelegate, ImmortalGCPolicy, type InvalidateAction, type InvalidateAllAction, LogoutManager, type Manager, type Mergeable, type Middleware, type MiddlewareAPI, type NI, type NetworkError, NetworkManager, type Normalize, type NormalizeNullable, type OptimisticAction, type PK, PollingSubscription, type Queryable, type ResetAction, ResetError, type ResolveType, type ResultEntry, type Schema, type SchemaArgs, type SchemaClass, type SetAction, type SetResponseAction, type SetResponseActionBase, type SetResponseActionError, type SetResponseActionSuccess, type State, type SubscribeAction, SubscriptionManager, type UnknownError, type UnsubscribeAction, type UpdateFunction, internal_d as __INTERNAL__, actionTypes_d as actionTypes, index_d as actions, applyManager, createReducer, initManager, initialState };
+export { type AbstractInstanceType, type ActionMeta, type ActionTypes, type ConnectionListener, Controller, type CreateCountRef, type DataClientDispatch, DefaultConnectionListener, type Denormalize, type DenormalizeNullable, type DevToolsConfig, DevToolsManager, type Dispatch, type EndpointExtraOptions, type EndpointInterface, type EndpointUpdateFunction, type EntityInterface, type ErrorTypes, type ExpireAllAction, ExpiryStatus, type FetchAction, type FetchFunction, type FetchMeta, type FetchingMeta, type GCAction, type GCInterface, type GCOptions, GCPolicy, type GenericDispatch, type INormalizeDelegate, type IQueryDelegate, ImmortalGCPolicy, type InvalidateAction, type InvalidateAllAction, LogoutManager, type Manager, type Mergeable, type Middleware, type MiddlewareAPI, type NI, type NetworkError, NetworkManager, type Normalize, type NormalizeNullable, type OptimisticAction, type PK, PollingSubscription, type Queryable, type ResetAction, ResetError, type ResolveType, type ResultEntry, type Schema, type SchemaArgs, type SchemaClass, type SetAction, type SetResponseAction, type SetResponseActionBase, type SetResponseActionError, type SetResponseActionSuccess, type State, type SubscribeAction, SubscriptionManager, type UnknownError, type UnsubscribeAction, type UpdateFunction, internal_d as __INTERNAL__, actionTypes_d as actionTypes, index_d as actions, applyManager, createReducer, initManager, initialState };