enhance: Experimental fetcher resolves before react render (#1046)

Author: ntucker

packages/core/src/state/NetworkManager.ts

@@ -99,8 +99,28 @@ export default class NetworkManager implements Manager {
   protected handleFetch(action: FetchAction, dispatch: Dispatch<any>) {
     const fetch = action.payload;
     const { key, throttle, resolve, reject } = action.meta;
-    const deferedFetch = () =>
-      fetch()
+
+    const deferedFetch = () => {
+      let promise = fetch();
+      const resolvePromise = (
+        promise: Promise<string | number | object | null>,
+      ) =>
+        promise
+          .then(data => {
+            resolve(data);
+            return data;
+          })
+          .catch(error => {
+            reject(error);
+            throw error;
+          });
+      // schedule non-throttled resolutions in a microtask before receive
+      // this enables users awaiting their fetch to trigger any react updates needed to deal
+      // with upcoming changes because of the fetch (for instance avoiding suspense if something is deleted)
+      if (!throttle && action.endpoint) {
+        promise = resolvePromise(promise);
+      }
+      promise = promise
         .then(data => {
           // does this throw if the reducer fails?
           dispatch(
@@ -123,14 +143,20 @@ export default class NetworkManager implements Manager {
           );
           throw error;
         });
-    let promise;
+      // legacy behavior schedules resolution after dispatch
+      if (!throttle && !action.endpoint) {
+        promise = resolvePromise(promise);
+      }
+      return promise;
+    };
+
     if (throttle) {
-      promise = this.throttle(key, deferedFetch);
+      return this.throttle(key, deferedFetch)
+        .then(data => resolve(data))
+        .catch(error => reject(error));
     } else {
-      promise = deferedFetch();
+      return deferedFetch().catch(() => {});
     }
-    promise.then(data => resolve(data)).catch(error => reject(error));
-    return promise;
   }
 
   /** Called when middleware intercepts a receive action.

packages/core/src/state/__tests__/networkManager.ts

@@ -92,6 +92,7 @@ describe('NetworkManager', () => {
       body: { id: 5, title: 'hi' },
       throttle: false,
     });
+    (fetchRejectAction.meta.promise as any).catch(e => {});
 
     let NM: NetworkManager;
     let middleware: Middleware;
@@ -278,7 +279,10 @@ describe('NetworkManager', () => {
           ...fetchRejectAction,
           meta: {
             ...fetchRejectAction.meta,
-            options: { errorExpiryLength: undefined },
+            options: {
+              ...fetchRejectAction.meta.options,
+              errorExpiryLength: undefined,
+            },
           },
         });
       } catch (error) {

packages/core/src/types.ts

@@ -7,6 +7,7 @@ import type {
   Schema,
   FetchFunction,
   EndpointExtraOptions,
+  EndpointInterface,
 } from '@rest-hooks/endpoint';
 import { ErrorableFSAWithPayloadAndMeta } from '@rest-hooks/core/fsa';
 import { FetchShape } from '@rest-hooks/core/endpoint/index';
@@ -130,6 +131,7 @@ export interface FetchAction<
     FetchMeta<any, any>
   > {
   meta: FetchMeta<Payload, S>;
+  endpoint?: EndpointInterface;
 }
 
 export interface SubscribeAction

packages/experimental/src/__tests__/useFetcher.ts

@@ -3,6 +3,8 @@ import nock from 'nock';
 import { useCache, useResource, ResolveType } from '@rest-hooks/core';
 
 // relative imports to avoid circular dependency in tsconfig references
+import { act } from '@testing-library/react-hooks';
+
 import {
   makeRenderRestHook,
   makeCacheProvider,
@@ -102,10 +104,13 @@ for (const makeProvider of [makeCacheProvider, makeExternalCacheProvider]) {
         };
       });
       expect(result.current.data).toBeUndefined();
-      const response = await result.current.fetch(
-        FutureArticleResource.detail(),
-        payload.id,
-      );
+      let response;
+      await act(async () => {
+        response = await result.current.fetch(
+          FutureArticleResource.detail(),
+          payload.id,
+        );
+      });
       expect(result.current.data).toBeDefined();
       expect(result.current.data?.content).toEqual(payload.content);
       expect(response).toEqual(payload);
@@ -153,11 +158,64 @@ for (const makeProvider of [makeCacheProvider, makeExternalCacheProvider]) {
           { resolverFixtures },
         );
         await waitForNextUpdate();
-        await result.current.fetch(FutureArticleResource.create(), {
-          id: 1,
+        await act(async () => {
+          await result.current.fetch(FutureArticleResource.create(), {
+            id: 1,
+          });
         });
+
         expect(result.current.articles.map(({ id }) => id)).toEqual([1, 5, 3]);
       }
     });
+
+    it('should not suspend once deleted and redirected at same time', async () => {
+      const temppayload = {
+        ...payload,
+        id: 1234,
+      };
+      mynock
+        .get(`/article-cooler/${temppayload.id}`)
+        .reply(200, temppayload)
+        .delete(`/article-cooler/${temppayload.id}`)
+        .reply(204, '');
+      const throws: Promise<any>[] = [];
+      const { result, waitForNextUpdate, rerender } = renderRestHook(
+        ({ id }) => {
+          try {
+            return [
+              useResource(FutureArticleResource.detail(), id) ?? null,
+              useFetcher(),
+            ] as const;
+          } catch (e) {
+            if (typeof e.then === 'function') {
+              if (e !== throws[throws.length - 1]) {
+                throws.push(e);
+              }
+            }
+            throw e;
+          }
+        },
+        { initialProps: { id: temppayload.id as number | null } },
+      );
+      expect(result.current).toBeUndefined();
+      await waitForNextUpdate();
+      const [data, fetch] = result.current;
+      expect(data).toBeInstanceOf(FutureArticleResource);
+      expect(data?.title).toBe(temppayload.title);
+      expect(throws.length).toBe(1);
+
+      mynock
+        .persist()
+        .get(`/article-cooler/${temppayload.id}`)
+        .reply(200, { ...temppayload, title: 'othertitle' });
+
+      expect(throws.length).toBe(1);
+      await act(async () => {
+        await fetch(FutureArticleResource.delete(), temppayload.id);
+        rerender({ id: null });
+      });
+      expect(throws.length).toBe(1);
+      expect(result.current[0]).toBe(null);
+    });
   });
 }

packages/experimental/src/createFetch.ts

@@ -47,6 +47,7 @@ export default function createFetch<
     type: actionTypes.FETCH_TYPE,
     payload: () => endpoint(...args),
     meta,
+    endpoint,
   };
 }
 

packages/experimental/src/rest/__tests__/Resource.test.ts

@@ -1,5 +1,6 @@
 import nock from 'nock';
 import { useResource } from '@rest-hooks/core';
+import { act } from '@testing-library/react-hooks';
 
 import Resource from '../Resource';
 import useFetcher from '../../useFetcher';
@@ -124,8 +125,10 @@ describe('Resource', () => {
       return { articles, nextPage, fetch };
     });
     await waitForNextUpdate();
-    await result.current.fetch(PaginatedArticleResource.listPage(), {
-      cursor: 2,
+    await act(async () => {
+      await result.current.fetch(PaginatedArticleResource.listPage(), {
+        cursor: 2,
+      });
     });
     expect(
       result.current.articles.map(
@@ -180,8 +183,10 @@ describe('Resource', () => {
     };
 
     mynock.get(`/complex-thing/5`).reply(200, secondResponse);
-    await result.current.fetch(ComplexResource.detail(), {
-      id: '5',
+    await act(async () => {
+      await result.current.fetch(ComplexResource.detail(), {
+        id: '5',
+      });
     });
     expect(result.current.article).toEqual({ ...secondResponse, extra: 'hi' });
   });