Skip to content

Commit

Permalink
Merge pull request #2965 from reduxjs/feature/1.9.1-more-fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
markerikson committed Nov 30, 2022
2 parents 87bebec + eaf7d5e commit a7ceaa2
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 19 deletions.
24 changes: 13 additions & 11 deletions packages/toolkit/src/createAsyncThunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion packages/toolkit/src/query/core/buildSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion packages/toolkit/src/query/endpointDefinitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down
55 changes: 50 additions & 5 deletions packages/toolkit/src/query/tests/createApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
},
}),
}),
})

Expand Down Expand Up @@ -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')
Expand All @@ -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),
})
})
})
18 changes: 17 additions & 1 deletion packages/toolkit/src/tests/createAsyncThunk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
getLog,
} from 'console-testing-library/pure'
import { expectType } from './helpers'
import { delay } from '../utils'

declare global {
interface Window {
Expand Down Expand Up @@ -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)
Expand All @@ -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
})

Expand Down

0 comments on commit a7ceaa2

Please sign in to comment.