Skip to content

Commit

Permalink
feat(createAsyncThunk): signal.reason contains the first argument pro…
Browse files Browse the repository at this point in the history
…vided to asyncThunkHandle.abort reduxjs#2395

Other changes:

fixes signal.aborted not set in AbortController shim
  • Loading branch information
FaberVitale committed Jun 16, 2022
1 parent b0f59fc commit b72d7f5
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 79 deletions.
12 changes: 10 additions & 2 deletions packages/toolkit/src/createAsyncThunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { createAction } from './createAction'
import type { ThunkDispatch } from 'redux-thunk'
import type { FallbackIfUnknown, IsAny, IsUnknown } from './tsHelpers'
import { nanoid } from './nanoid'
import type { AbortSignalWithReason } from './function-utils'
import { abortControllerWithReason } from './function-utils'

// @ts-ignore we need the import of these types due to a bundling issue.
type _Keep = PayloadAction | ActionCreatorWithPreparedPayload<any, unknown>
Expand Down Expand Up @@ -541,7 +543,7 @@ export function createAsyncThunk<
reason: undefined,
throwIfAborted() {},
}
abort() {
abort(reason?: any) {
if (process.env.NODE_ENV !== 'production') {
if (!displayedWarning) {
displayedWarning = true
Expand All @@ -551,6 +553,12 @@ If you want to use the AbortController to react to \`abort\` events, please cons
)
}
}

if (!this.signal.aborted) {
this.signal.aborted = true
;(this.signal as AbortSignalWithReason<typeof reason>).reason =
reason
}
}
}

Expand All @@ -575,7 +583,7 @@ If you want to use the AbortController to react to \`abort\` events, please cons
function abort(reason?: string) {
if (started) {
abortReason = reason
abortController.abort()
abortControllerWithReason(abortController, reason)
}
}

Expand Down
43 changes: 43 additions & 0 deletions packages/toolkit/src/function-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* @internal
* At the time of writing `lib.dom.ts` does not provide `abortSignal.reason`.
*/
export type AbortSignalWithReason<T> = AbortSignal & { reason?: T }

/**
* Calls `abortController.abort(reason)` and patches `signal.reason`.
* if it is not supported.
*
* At the time of writing `signal.reason` is available in FF chrome, edge node 17 and deno.
* @param abortController
* @param reason
* @returns
* @see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason
*/
export const abortControllerWithReason = <T>(
abortController: AbortController,
reason: T
): void => {
type Consumer<T> = (val: T) => void

const signal = abortController.signal as AbortSignalWithReason<T>

if (signal.aborted) {
return
}

// Patch `reason` if necessary.
// - We use defineProperty here because reason is a getter of `AbortSignal.__proto__`.
// - We need to patch 'reason' before calling `.abort()` because listeners to the 'abort'
// event are are notified immediately.
if (!('reason' in signal)) {
Object.defineProperty(signal, 'reason', {
enumerable: true,
value: reason,
configurable: true,
writable: true,
})
}

;(abortController.abort as Consumer<typeof reason>)(reason)
}
11 changes: 3 additions & 8 deletions packages/toolkit/src/listenerMiddleware/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import type { Dispatch, AnyAction, MiddlewareAPI } from 'redux'
import type { ThunkDispatch } from 'redux-thunk'
import { createAction } from '../createAction'
import { nanoid } from '../nanoid'

import { abortControllerWithReason } from '../function-utils'
import type { AbortSignalWithReason } from '../function-utils'
import type {
ListenerMiddleware,
ListenerMiddlewareInstance,
Expand All @@ -21,15 +22,9 @@ import type {
ForkedTask,
TypedRemoveListener,
TaskResult,
AbortSignalWithReason,
UnsubscribeListenerOptions,
} from './types'
import {
abortControllerWithReason,
addAbortSignalListener,
assertFunction,
catchRejection,
} from './utils'
import { addAbortSignalListener, assertFunction, catchRejection } from './utils'
import {
listenerCancelled,
listenerCompleted,
Expand Down
3 changes: 2 additions & 1 deletion packages/toolkit/src/listenerMiddleware/task.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TaskAbortError } from './exceptions'
import type { AbortSignalWithReason, TaskResult } from './types'
import type { TaskResult } from './types'
import type { AbortSignalWithReason } from '../function-utils'
import { addAbortSignalListener, catchRejection } from './utils'

/**
Expand Down
8 changes: 2 additions & 6 deletions packages/toolkit/src/listenerMiddleware/tests/fork.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import type { EnhancedStore } from '@reduxjs/toolkit'
import { configureStore, createSlice, createAction } from '@reduxjs/toolkit'

import type { AbortSignalWithReason } from '../../function-utils'
import type { PayloadAction } from '@reduxjs/toolkit'
import type {
AbortSignalWithReason,
ForkedTaskExecutor,
TaskResult,
} from '../types'
import type { ForkedTaskExecutor, TaskResult } from '../types'
import { createListenerMiddleware, TaskAbortError } from '../index'
import {
listenerCancelled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@ import type {
UnsubscribeListener,
ListenerMiddleware,
} from '../index'
import type {
AbortSignalWithReason,
AddListenerOverloads,
TypedRemoveListener,
} from '../types'
import type { AbortSignalWithReason } from '../../function-utils'
import type { AddListenerOverloads, TypedRemoveListener } from '../types'
import { listenerCancelled, listenerCompleted } from '../exceptions'

const middlewareApi = {
Expand Down Expand Up @@ -185,7 +182,7 @@ describe('createListenerMiddleware', () => {
middleware: (gDM) => gDM().prepend(listenerMiddleware.middleware),
})

let foundExtra = null
let foundExtra: number | null = null

const typedAddListener =
listenerMiddleware.startListening as TypedStartListening<
Expand Down Expand Up @@ -1122,31 +1119,34 @@ describe('createListenerMiddleware', () => {
expect(takeResult).toEqual([increment(), stateCurrent, stateBefore])
})

test("take resolves to `[A, CurrentState, PreviousState] | null` if a possibly undefined timeout parameter is provided", async () => {
test('take resolves to `[A, CurrentState, PreviousState] | null` if a possibly undefined timeout parameter is provided', async () => {
const store = configureStore({
reducer: counterSlice.reducer,
middleware: (gDM) => gDM().prepend(middleware),
})

type ExpectedTakeResultType = readonly [ReturnType<typeof increment>, CounterState, CounterState] | null
type ExpectedTakeResultType =
| readonly [ReturnType<typeof increment>, CounterState, CounterState]
| null

let timeout: number | undefined = undefined
let done = false

const startAppListening = startListening as TypedStartListening<CounterState>
const startAppListening =
startListening as TypedStartListening<CounterState>
startAppListening({
predicate: incrementByAmount.match,
effect: async (_, listenerApi) => {
const stateBefore = listenerApi.getState()

let takeResult = await listenerApi.take(increment.match, timeout)
const stateCurrent = listenerApi.getState()
expect(takeResult).toEqual([increment(), stateCurrent, stateBefore])

timeout = 1
takeResult = await listenerApi.take(increment.match, timeout)
expect(takeResult).toBeNull()

expectType<ExpectedTakeResultType>(takeResult)

done = true
Expand All @@ -1156,7 +1156,7 @@ describe('createListenerMiddleware', () => {
store.dispatch(increment())

await delay(25)
expect(done).toBe(true);
expect(done).toBe(true)
})

test('condition method resolves promise when the predicate succeeds', async () => {
Expand Down
12 changes: 3 additions & 9 deletions packages/toolkit/src/listenerMiddleware/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,6 @@ import type {
import type { ThunkDispatch } from 'redux-thunk'
import type { TaskAbortError } from './exceptions'

/**
* @internal
* At the time of writing `lib.dom.ts` does not provide `abortSignal.reason`.
*/
export type AbortSignalWithReason<T> = AbortSignal & { reason?: T }

/**
* Types copied from RTK
*/
Expand Down Expand Up @@ -177,9 +171,9 @@ export interface ListenerEffectAPI<
* rejects if the listener has been cancelled or is completed.
*
* The return value is `true` if the predicate succeeds or `false` if a timeout is provided and expires first.
*
*
* ### Example
*
*
* ```ts
* const updateBy = createAction<number>('counter/updateBy');
*
Expand All @@ -201,7 +195,7 @@ export interface ListenerEffectAPI<
*
* The return value is the `[action, currentState, previousState]` combination that the predicate saw as arguments.
*
* The promise resolves to null if a timeout is provided and expires first,
* The promise resolves to null if a timeout is provided and expires first,
*
* ### Example
*
Expand Down
40 changes: 0 additions & 40 deletions packages/toolkit/src/listenerMiddleware/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import type { AbortSignalWithReason } from './types'

export const assertFunction: (
func: unknown,
expected: string
Expand Down Expand Up @@ -29,41 +27,3 @@ export const addAbortSignalListener = (
) => {
abortSignal.addEventListener('abort', callback, { once: true })
}

/**
* Calls `abortController.abort(reason)` and patches `signal.reason`.
* if it is not supported.
*
* At the time of writing `signal.reason` is available in FF chrome, edge node 17 and deno.
* @param abortController
* @param reason
* @returns
* @see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason
*/
export const abortControllerWithReason = <T>(
abortController: AbortController,
reason: T
): void => {
type Consumer<T> = (val: T) => void

const signal = abortController.signal as AbortSignalWithReason<T>

if (signal.aborted) {
return
}

// Patch `reason` if necessary.
// - We use defineProperty here because reason is a getter of `AbortSignal.__proto__`.
// - We need to patch 'reason' before calling `.abort()` because listeners to the 'abort'
// event are are notified immediately.
if (!('reason' in signal)) {
Object.defineProperty(signal, 'reason', {
enumerable: true,
value: reason,
configurable: true,
writable: true,
})
}

;(abortController.abort as Consumer<typeof reason>)(reason)
}
70 changes: 70 additions & 0 deletions 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 type { AbortSignalWithReason } from '../function-utils'

declare global {
interface Window {
Expand Down Expand Up @@ -426,6 +427,47 @@ describe('createAsyncThunk with abortController', () => {
)
})

test('signal.reason contains the first argument provided to asyncThunkHandle.abort', async () => {
const rejectionReason = 'custom-abort-reason'
let apiSignal: AbortSignalWithReason<typeof rejectionReason> | undefined

const signalReasonAsyncThunk = createAsyncThunk(
'test-signal-reason',
function abortablePayloadCreator(_: any, { signal }) {
apiSignal = signal
return new Promise((resolve, reject) => {
if (signal.aborted) {
reject(
new DOMException(
'This should never be reached as it should already be handled.',
'AbortError'
)
)
}
signal.addEventListener('abort', () => {
reject((signal as NonNullable<typeof apiSignal>).reason)
})
setTimeout(resolve, 10)
})
}
)

const asyncThunkHandle = store.dispatch(signalReasonAsyncThunk({}))

expect(apiSignal).toHaveProperty(['aborted'], false)
expect(apiSignal).not.toHaveProperty(['reason'], rejectionReason)

asyncThunkHandle.abort(rejectionReason)

const result = await asyncThunkHandle

// calling unwrapResult on the returned object re-throws the error from the abortablePayloadCreator
expect(() => unwrapResult(result)).toThrowError()

expect(apiSignal).toHaveProperty(['aborted'], true)
expect(apiSignal).toHaveProperty(['reason'], rejectionReason)
})

test('even when the payloadCreator does not directly support the signal, no further actions are dispatched', async () => {
const unawareAsyncThunk = createAsyncThunk('unaware', async () => {
await new Promise((resolve) => setTimeout(resolve, 100))
Expand Down Expand Up @@ -520,6 +562,34 @@ describe('createAsyncThunk with abortController', () => {
If you want to use the AbortController to react to \`abort\` events, please consider importing a polyfill like 'abortcontroller-polyfill/dist/abortcontroller-polyfill-only'."
`)
})

test('signal.reason contains the first argument provided to asyncThunkHandle.abort', async () => {
const rejectionReason = 'custom-abort-reason'
let apiSignal: AbortSignalWithReason<typeof rejectionReason> | undefined

const asyncThunk = freshlyLoadedModule.createAsyncThunk(
'longRunning',
async (_: unknown, { signal }) => {
await new Promise((resolve) => {
apiSignal = signal

setTimeout(resolve, 10)
})
}
)

const asyncThunkHandle = store.dispatch(asyncThunk({}))

expect(apiSignal).toHaveProperty(['aborted'], false)
expect(apiSignal).not.toHaveProperty(['reason'], rejectionReason)

asyncThunkHandle.abort(rejectionReason)

const result = await asyncThunkHandle

expect(apiSignal).toHaveProperty(['aborted'], true)
expect(apiSignal).toHaveProperty(['reason'], rejectionReason)
})
})
})

Expand Down

0 comments on commit b72d7f5

Please sign in to comment.