fix: Interceptors work on manager-dispatched actions (#3365)

Author: ntucker

.changeset/silly-flies-juggle.md

@@ -0,0 +1,9 @@
+---
+'@data-client/test': patch
+---
+
+fix: Interceptors work on manager-dispatched actions.
+
+For example, renderHook now can use resolverFixtures to resolve fetches for subscriptions.
+This was not possible before as SubscriptionManager's dispatches would not be intercepted with the
+previous implementation.

.changeset/sour-walls-kick.md

@@ -0,0 +1,8 @@
+---
+'@data-client/core': patch
+---
+
+internal: Controller.bindMiddleware() to be used in applyMiddleware.
+
+This API is not intended to be used elsewhere, but will become the standard interface between
+Controller's and applyMiddleware.

packages/core/src/controller/Controller.ts

@@ -35,16 +35,19 @@ import {
 } from './actions/index.js';
 import ensurePojo from './ensurePojo.js';
 import type { EndpointUpdateFunction } from './types.js';
+import { ReduxMiddlewareAPI } from '../manager/applyManager.js';
 import type { GCInterface } from '../state/GCPolicy.js';
 import { ImmortalGCPolicy } from '../state/GCPolicy.js';
 import { initialState } from '../state/reducer/createReducer.js';
 import selectMeta from '../state/selectMeta.js';
-import type { ActionTypes, State } from '../types.js';
+import type { ActionTypes, Dispatch, State } from '../types.js';
 
 export type GenericDispatch = (value: any) => Promise<void>;
 export type DataClientDispatch = (value: ActionTypes) => Promise<void>;
 
-interface ConstructorProps<D extends GenericDispatch = DataClientDispatch> {
+export interface ControllerConstructorProps<
+  D extends GenericDispatch = DataClientDispatch,
+> {
   dispatch?: D;
   getState?: () => State<unknown>;
   memo?: Pick<MemoCache, 'denormalize' | 'query' | 'buildQueryKey'>;
@@ -68,22 +71,23 @@ const unsetState = (): State<unknown> => {
  * @see https://dataclient.io/docs/api/Controller
  */
 export default class Controller<
+  // NOTE: We template on entire dispatch, so we can be contravariant on ActionTypes
   D extends GenericDispatch = DataClientDispatch,
 > {
   /**
    * Dispatches an action to Reactive Data Client reducer.
    *
    * @see https://dataclient.io/docs/api/Controller#dispatch
    */
-  declare readonly dispatch: D;
+  declare protected _dispatch: D;
   /**
    * Gets the latest state snapshot that is fully committed.
    *
    * This can be useful for imperative use-cases like event handlers.
    * This should *not* be used to render; instead useSuspense() or useCache()
    * @see https://dataclient.io/docs/api/Controller#getState
    */
-  declare readonly getState: () => State<unknown>;
+  declare getState: () => State<unknown>;
   /**
    * Singleton to maintain referential equality between calls
    */
@@ -102,13 +106,35 @@ export default class Controller<
     getState = unsetState,
     memo = new MemoCache(),
     gcPolicy = new ImmortalGCPolicy(),
-  }: ConstructorProps<D> = {}) {
-    this.dispatch = dispatch;
+  }: ControllerConstructorProps<D> = {}) {
+    this._dispatch = dispatch;
     this.getState = getState;
     this.memo = memo;
     this.gcPolicy = gcPolicy;
   }
 
+  // TODO: drop when drop support for destructuring (0.14 and below)
+  set dispatch(dispatch: D) {
+    /* istanbul ignore next */
+    this._dispatch = dispatch;
+  }
+
+  // TODO: drop when drop support for destructuring (0.14 and below)
+  get dispatch(): D {
+    return this._dispatch;
+  }
+
+  bindMiddleware({
+    dispatch,
+    getState,
+  }: {
+    dispatch: D;
+    getState: ReduxMiddlewareAPI['getState'];
+  }) {
+    this._dispatch = dispatch;
+    this.getState = getState;
+  }
+
   /*************** Action Dispatchers ***************/
 
   /**

packages/core/src/controller/__tests__/Controller.ts

@@ -55,14 +55,15 @@ describe('Controller', () => {
         },
       };
       const getState = () => state;
+      const dispatch = jest.fn(() => Promise.resolve());
       const controller = new Controller({
-        dispatch: jest.fn(() => Promise.resolve()),
+        dispatch,
         getState,
       });
       const article = await controller.fetchIfStale(CoolerArticleResource.get, {
         id: payload.id,
       });
-      expect(controller.dispatch.mock.calls.length).toBe(0);
+      expect(dispatch.mock.calls.length).toBe(0);
       expect(article.title).toBe(payload.title);
     });
     it('should fetch if result stale', () => {
@@ -92,15 +93,16 @@ describe('Controller', () => {
         },
       };
       const getState = () => state;
+      const dispatch = jest.fn(() => Promise.resolve());
       const controller = new Controller({
-        dispatch: jest.fn(() => Promise.resolve()),
+        dispatch,
         getState,
       });
       controller.fetchIfStale(CoolerArticleResource.get, {
         id: payload.id,
       });
 
-      expect(controller.dispatch.mock.calls.length).toBe(1);
+      expect(dispatch.mock.calls.length).toBe(1);
     });
   });
 });

packages/core/src/manager/applyManager.ts

@@ -18,14 +18,13 @@ export default function applyManager(
   }
   return managers.map((manager, i) => {
     if (!manager.middleware) manager.middleware = manager.getMiddleware?.();
-    return ({ dispatch, getState }) => {
+    return (api: ReduxMiddlewareAPI) => {
       if (i === 0) {
-        (controller as any).dispatch = dispatch;
-        (controller as any).getState = getState;
+        controller.bindMiddleware(api);
       }
       // controller is a superset of the middleware API
       return (manager as Manager & { middleware: ReduxMiddleware }).middleware(
-        controller as Controller<any>,
+        controller as any,
       );
     };
   });

packages/endpoint/src/__tests__/endpoint.ts

@@ -1,6 +1,6 @@
 import nock from 'nock';
 
-import type { default as TEndpoint, EndpointInstance } from '../endpoint';
+import type { default as TEndpoint } from '../endpoint';
 import { EndpointInterface } from '../interface';
 import Entity from '../schemas/Entity';
 

packages/react/src/__tests__/integration-garbage-collection.web.tsx

@@ -6,16 +6,21 @@ import {
 } from '@data-client/react';
 import { MockResolver } from '@data-client/test';
 import { render, screen, act } from '@testing-library/react';
-import { ArticleResource } from '__tests__/new';
+import { Article, ArticleResource } from '__tests__/new';
 import '@testing-library/jest-dom';
 import { useState } from 'react';
 
 const mockGetList = jest.fn();
 const mockGet = jest.fn();
 const GC_INTERVAL = 5000;
 
-const ListView = ({ onSelect }: { onSelect: (id: number) => void }) => {
-  const articles = useSuspense(ArticleResource.getList);
+const ArticleList = ({
+  articles,
+  onSelect,
+}: {
+  articles: Article[];
+  onSelect: (id: number) => void;
+}) => {
   return (
     <div>
       {articles.map(article => (
@@ -26,18 +31,30 @@ const ListView = ({ onSelect }: { onSelect: (id: number) => void }) => {
     </div>
   );
 };
+const ArticleDetail = ({ article }: { article: Article }) => {
+  return (
+    <div>
+      <h3>{article.title}</h3>
+      <p>{article.content}</p>
+    </div>
+  );
+};
+
+const ListView = ({ onSelect }: { onSelect: (id: number) => void }) => {
+  const articles = useSuspense(ArticleResource.getList);
+  return <ArticleList articles={articles} onSelect={onSelect} />;
+};
 
 const DetailView = ({ id }: { id: number }) => {
   const article = useSuspense(ArticleResource.get, { id });
   const [toggle, setToggle] = useState(false);
 
   return (
-    <div>
-      <h3>{article.title}</h3>
-      <p>{article.content}</p>
+    <>
+      <ArticleDetail article={article} />{' '}
       <button onClick={() => setToggle(!toggle)}>Toggle Re-render</button>
       {toggle && <div>Toggle state: {toggle.toString()}</div>}
-    </div>
+    </>
   );
 };
 

packages/react/src/components/__tests__/provider.native.tsx

@@ -89,8 +89,10 @@ describe('<DataProvider />', () => {
 
   it('should not change dispatch function on re-render', () => {
     let dispatch;
+    let controller;
     let count = 0;
     function DispatchTester() {
+      controller = useController();
       dispatch = useController().dispatch;
       count++;
       return null;
@@ -100,6 +102,7 @@ describe('<DataProvider />', () => {
     const { rerender } = render(tree);
     expect(dispatch).toBeDefined();
     let curDisp = dispatch;
+    let curController = controller;
     rerender(tree);
     expect(curDisp).toBe(dispatch);
     expect(count).toBe(1);
@@ -110,6 +113,7 @@ describe('<DataProvider />', () => {
     rerender(<DataProvider managers={managers}>{chil}</DataProvider>);
     expect(count).toBe(1);
     curDisp = dispatch;
+    curController = controller;
     rerender(<DataProvider managers={managers}>{chil}</DataProvider>);
     expect(curDisp).toBe(dispatch);
     expect(count).toBe(1);
@@ -120,22 +124,23 @@ describe('<DataProvider />', () => {
         {chil}
       </ControllerContext.Provider>,
     );
-    expect(curDisp).not.toBe(dispatch);
+    expect(curController).not.toBe(controller);
     expect(count).toBe(2);
   });
+
   it('should change state', () => {
-    let dispatch: any, state;
+    let controller: any, state;
     let count = 0;
     function ContextTester() {
-      dispatch = useController().dispatch;
+      controller = useController();
       state = useContext(StateContext);
       count++;
       return null;
     }
     const chil = <ContextTester />;
     const tree = <DataProvider>{chil}</DataProvider>;
     render(tree);
-    expect(dispatch).toBeDefined();
+    expect(controller.dispatch).toBeDefined();
     expect(state).toBeDefined();
     const action: SetResponseAction = {
       type: SET_RESPONSE,
@@ -150,7 +155,7 @@ describe('<DataProvider />', () => {
       },
     };
     act(() => {
-      dispatch(action);
+      controller.dispatch(action);
     });
     expect(count).toBe(2);
     expect(state).toMatchSnapshot();

packages/react/src/components/__tests__/provider.tsx

@@ -85,8 +85,10 @@ describe('<DataProvider />', () => {
 
   it('should not change dispatch function on re-render', () => {
     let dispatch;
+    let controller;
     let count = 0;
     function DispatchTester() {
+      controller = useController();
       dispatch = useController().dispatch;
       count++;
       return null;
@@ -96,6 +98,7 @@ describe('<DataProvider />', () => {
     const { rerender } = render(tree);
     expect(dispatch).toBeDefined();
     let curDisp = dispatch;
+    let curController = controller;
     rerender(tree);
     expect(curDisp).toBe(dispatch);
     expect(count).toBe(1);
@@ -106,6 +109,7 @@ describe('<DataProvider />', () => {
     rerender(<DataProvider managers={managers}>{chil}</DataProvider>);
     expect(count).toBe(1);
     curDisp = dispatch;
+    curController = controller;
     rerender(<DataProvider managers={managers}>{chil}</DataProvider>);
     expect(curDisp).toBe(dispatch);
     expect(count).toBe(1);
@@ -116,7 +120,7 @@ describe('<DataProvider />', () => {
         {chil}
       </ControllerContext.Provider>,
     );
-    expect(curDisp).not.toBe(dispatch);
+    expect(curController).not.toBe(controller);
     expect(count).toBe(2);
   });