Merge pull request #2965 from reduxjs/feature/1.9.1-more-fixes

Author: markerikson

packages/toolkit/src/createAsyncThunk.ts

@@ -590,18 +590,10 @@ If you want to use the AbortController to react to \`abort\` events, please cons
         const abortController = new AC()
         let abortReason: string | undefined
 
-        const abortedPromise = new Promise<never>((_, reject) =>
-          abortController.signal.addEventListener('abort', () =>
-            reject({ name: 'AbortError', message: abortReason || 'Aborted' })
-          )
-        )
-
         let started = false
         function abort(reason?: string) {
-          if (started) {
-            abortReason = reason
-            abortController.abort()
-          }
+          abortReason = reason
+          abortController.abort()
         }
 
         const promise = (async function () {
@@ -611,14 +603,24 @@ If you want to use the AbortController to react to \`abort\` events, please cons
             if (isThenable(conditionResult)) {
               conditionResult = await conditionResult
             }
-            if (conditionResult === false) {
+
+            if (conditionResult === false || abortController.signal.aborted) {
               // eslint-disable-next-line no-throw-literal
               throw {
                 name: 'ConditionError',
                 message: 'Aborted due to condition callback returning false.',
               }
             }
             started = true
+
+            const abortedPromise = new Promise<never>((_, reject) =>
+              abortController.signal.addEventListener('abort', () =>
+                reject({
+                  name: 'AbortError',
+                  message: abortReason || 'Aborted',
+                })
+              )
+            )
             dispatch(
               pending(
                 requestId,

packages/toolkit/src/query/core/buildSlice.ts

@@ -181,6 +181,8 @@ export function buildSlice({
 
               if (merge) {
                 if (substate.data !== undefined) {
+                  const { fulfilledTimeStamp, arg, baseQueryMeta, requestId } =
+                    meta
                   // There's existing cache data. Let the user merge it in themselves.
                   // We're already inside an Immer-powered reducer, and the user could just mutate `substate.data`
                   // themselves inside of `merge()`. But, they might also want to return a new value.
@@ -189,7 +191,12 @@ export function buildSlice({
                     substate.data,
                     (draftSubstateData) => {
                       // As usual with Immer, you can mutate _or_ return inside here, but not both
-                      return merge(draftSubstateData, payload)
+                      return merge(draftSubstateData, payload, {
+                        arg: arg.originalArgs,
+                        baseQueryMeta,
+                        fulfilledTimeStamp,
+                        requestId,
+                      })
                     }
                   )
                   substate.data = newData

packages/toolkit/src/query/endpointDefinitions.ts

@@ -445,7 +445,13 @@ export interface QueryExtraOptions<
    */
   merge?(
     currentCacheData: ResultType,
-    responseData: ResultType
+    responseData: ResultType,
+    otherArgs: {
+      arg: QueryArg
+      baseQueryMeta: BaseQueryMeta<BaseQuery>
+      requestId: string
+      fulfilledTimeStamp: number
+    }
   ): ResultType | void
 
   /**

packages/toolkit/src/query/tests/createApi.test.ts

@@ -35,6 +35,11 @@ afterAll(() => {
   spy.mockRestore()
 })
 
+function paginate<T>(array: T[], page_size: number, page_number: number) {
+  // human-readable page numbers usually start with 1, so we reduce 1 in the first argument
+  return array.slice((page_number - 1) * page_size, page_number * page_size)
+}
+
 test('sensible defaults', () => {
   const api = createApi({
     baseQuery: fetchBaseQuery(),
@@ -923,6 +928,22 @@ describe('custom serializeQueryArgs per endpoint', () => {
           return currentArg !== previousArg
         },
       }),
+      listItems2: build.query<{ items: string[]; meta?: any }, number>({
+        query: (pageNumber) => `/listItems2?page=${pageNumber}`,
+        serializeQueryArgs: ({ endpointName }) => {
+          return endpointName
+        },
+        transformResponse(items: string[]) {
+          return { items }
+        },
+        merge: (currentCache, newData, meta) => {
+          currentCache.items.push(...newData.items)
+          currentCache.meta = meta
+        },
+        forceRefetch({ currentArg, previousArg }) {
+          return currentArg !== previousArg
+        },
+      }),
     }),
   })
 
@@ -1010,11 +1031,6 @@ describe('custom serializeQueryArgs per endpoint', () => {
     const allItems = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'i']
     const PAGE_SIZE = 3
 
-    function paginate<T>(array: T[], page_size: number, page_number: number) {
-      // human-readable page numbers usually start with 1, so we reduce 1 in the first argument
-      return array.slice((page_number - 1) * page_size, page_number * page_size)
-    }
-
     server.use(
       rest.get('https://example.com/listItems', (req, res, ctx) => {
         const pageString = req.url.searchParams.get('page')
@@ -1038,4 +1054,33 @@ describe('custom serializeQueryArgs per endpoint', () => {
     const updatedEntry = selectListItems(storeRef.store.getState())
     expect(updatedEntry.data).toEqual(['a', 'b', 'c', 'd', 'e', 'f'])
   })
+
+  test('merge receives a meta object as an argument', async () => {
+    const allItems = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'i']
+    const PAGE_SIZE = 3
+
+    server.use(
+      rest.get('https://example.com/listItems2', (req, res, ctx) => {
+        const pageString = req.url.searchParams.get('page')
+        const pageNum = parseInt(pageString || '0')
+
+        const results = paginate(allItems, PAGE_SIZE, pageNum)
+        return res(ctx.json(results))
+      })
+    )
+
+    const selectListItems = api.endpoints.listItems2.select(0)
+
+    await storeRef.store.dispatch(api.endpoints.listItems2.initiate(1))
+    await storeRef.store.dispatch(api.endpoints.listItems2.initiate(2))
+    const cacheEntry = selectListItems(storeRef.store.getState())
+
+    // Should have passed along the third arg from `merge` containing these fields
+    expect(cacheEntry.data?.meta).toEqual({
+      requestId: expect.any(String),
+      fulfilledTimeStamp: expect.any(Number),
+      arg: 2,
+      baseQueryMeta: expect.any(Object),
+    })
+  })
 })

packages/toolkit/src/tests/createAsyncThunk.test.ts

@@ -13,6 +13,7 @@ import {
   getLog,
 } from 'console-testing-library/pure'
 import { expectType } from './helpers'
+import { delay } from '../utils'
 
 declare global {
   interface Window {
@@ -621,6 +622,21 @@ describe('conditional skipping of asyncThunks', () => {
     )
   })
 
+  test('async condition with AbortController signal first', async () => {
+    const condition = async () => {
+      await delay(25)
+      return true
+    }
+    const asyncThunk = createAsyncThunk('test', payloadCreator, { condition })
+
+    try {
+      const thunkPromise = asyncThunk(arg)(dispatch, getState, extra)
+      thunkPromise.abort()
+      await thunkPromise
+    } catch (err) {}
+    expect(dispatch).toHaveBeenCalledTimes(0)
+  })
+
   test('rejected action is not dispatched by default', async () => {
     const asyncThunk = createAsyncThunk('test', payloadCreator, { condition })
     await asyncThunk(arg)(dispatch, getState, extra)
@@ -630,7 +646,7 @@ describe('conditional skipping of asyncThunks', () => {
 
   test('does not fail when attempting to abort a canceled promise', async () => {
     const asyncPayloadCreator = jest.fn(async (x: typeof arg) => {
-      await new Promise((resolve) => setTimeout(resolve, 2000))
+      await delay(200)
       return 10
     })