enhance: handle race conditions with similar requests

reactive · Jan 19, 2020 · be79bf6 · be79bf6
1 parent c6f9ec7
commit be79bf6
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 3 deletions.
diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md
@@ -9,7 +9,7 @@
 
 ## Soon
 
-- [ ] Optimistic query update on create
+- [x] Optimistic query update on create
 - [x] Optional redux-integration
 - [x] Polling based subscriptions
 - [ ] Automatic batching

diff --git a/packages/rest-hooks/src/react-integration/__tests__/integration.tsx b/packages/rest-hooks/src/react-integration/__tests__/integration.tsx
@@ -385,6 +385,104 @@ for (const makeProvider of [makeCacheProvider, makeExternalCacheProvider]) {
           }),
         ]);
       });
+
+      it('should clear only earlier optimistic updates when a promise resolves', async () => {
+        jest.useFakeTimers();
+
+        const params = { id: payload.id };
+        const { result, waitForNextUpdate } = renderRestHook(
+          () => {
+            const put = useFetcher(CoolerArticleResource.partialUpdateShape());
+            const article = useCache(
+              CoolerArticleResource.detailShape(),
+              params,
+            );
+            return { put, article };
+          },
+          {
+            results: [
+              {
+                request: CoolerArticleResource.detailShape(),
+                params,
+                result: payload,
+              },
+            ],
+          },
+        );
+
+        // first optimistic
+        mynock
+          .patch('/article-cooler/5')
+          .delay(200)
+          .reply(200, {
+            ...payload,
+            title: 'first',
+            content: 'first',
+          });
+        result.current.put(params, {
+          title: 'firstoptimistic',
+          content: 'firstoptimistic',
+        });
+        expect(result.current.article).toEqual(
+          CoolerArticleResource.fromJS({
+            ...payload,
+            title: 'firstoptimistic',
+            content: 'firstoptimistic',
+          }),
+        );
+
+        // second optimistic
+        mynock
+          .patch('/article-cooler/5')
+          .delay(50)
+          .reply(200, {
+            ...payload,
+            title: 'second',
+          });
+        result.current.put(params, {
+          title: 'secondoptimistic',
+        });
+        expect(result.current.article).toEqual(
+          CoolerArticleResource.fromJS({
+            ...payload,
+            title: 'secondoptimistic',
+            content: 'firstoptimistic',
+          }),
+        );
+
+        // second optimistic
+        mynock
+          .patch('/article-cooler/5')
+          .delay(500)
+          .reply(200, {
+            ...payload,
+            tags: ['third'],
+          });
+        result.current.put(params, {
+          tags: ['thirdoptimistic'],
+        });
+        expect(result.current.article).toEqual(
+          CoolerArticleResource.fromJS({
+            ...payload,
+            title: 'secondoptimistic',
+            content: 'firstoptimistic',
+            tags: ['thirdoptimistic'],
+          }),
+        );
+
+        // resolve second request while first is in flight
+        jest.advanceTimersByTime(51);
+        await waitForNextUpdate();
+
+        // first and second optimistic should be cleared with only third optimistic left to be layerd
+        // on top of second's network response
+        expect(result.current.article).toEqual(
+          CoolerArticleResource.fromJS({
+            ...payload,
+            title: 'second',
+          }),
+        );
+      });
     });
   });
 }
diff --git a/packages/rest-hooks/src/state/__tests__/reducer.ts b/packages/rest-hooks/src/state/__tests__/reducer.ts
@@ -86,6 +86,7 @@ describe('reducer', () => {
       meta: {
         schema: ArticleResource.getEntitySchema(),
         url: ArticleResource.listUrl(payload),
+        date: 0,
       },
     };
     const iniState = {
@@ -102,6 +103,7 @@ describe('reducer', () => {
       meta: {
         schema: ArticleResource.getEntitySchema(),
         url: id.toString(),
+        date: 0,
       },
     };
     const iniState: any = {
@@ -235,6 +237,7 @@ describe('reducer', () => {
             schema: ArticleResource.getEntitySchema(),
             url: ArticleResource.createShape().getFetchKey({}),
             updaters,
+            date: 0,
           },
         };
       }
@@ -381,6 +384,7 @@ describe('reducer', () => {
       meta: {
         schema: ArticleResource.getEntitySchema(),
         url: ArticleResource.url({ id }),
+        date: 0,
       },
       error: true,
     };
@@ -397,6 +401,7 @@ describe('reducer', () => {
       meta: {
         schema: ArticleResource.getEntitySchema(),
         url: ArticleResource.url({ id }),
+        date: 0,
       },
       error: true,
     };

diff --git a/packages/rest-hooks/src/state/reducer.ts b/packages/rest-hooks/src/state/reducer.ts
@@ -142,9 +142,15 @@ export default function reducer(
 
 type Writable<T> = { [P in keyof T]: NonNullable<T[P]> };
 
-function filterOptimistic(state: State<unknown>, action: ResponseActions) {
+/** Filter all requests with same serialization that did not start after the resolving request */
+function filterOptimistic(
+  state: State<unknown>,
+  resolvingAction: ResponseActions,
+) {
   return state.optimistic.filter(
-    optimisticAction => optimisticAction.meta.url !== action.meta.url,
+    optimisticAction =>
+      optimisticAction.meta.url !== resolvingAction.meta.url ||
+      optimisticAction.meta.date > resolvingAction.meta.date,
   );
 }
 

diff --git a/packages/rest-hooks/src/types.ts b/packages/rest-hooks/src/types.ts
@@ -74,6 +74,7 @@ export type ReceiveAction<
 interface RPCMeta<S extends Schema> {
   schema: S;
   url: string;
+  date: number;
   updaters?: { [key: string]: UpdateFunction<S, any> };
 }
 
@@ -89,6 +90,7 @@ export type RPCAction<
 interface PurgeMeta {
   schema: schemas.EntityInterface<any>;
   url: string;
+  date: number;
 }
 
 export type PurgeAction = ErrorableFSAWithMeta<