Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional 1.9.1 fixes #2965

Merged
merged 2 commits into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 13 additions & 11 deletions packages/toolkit/src/createAsyncThunk.ts
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
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
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
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
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