Skip to content

add .abort() to the createAsyncThunk thunkAction #362

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

Merged
merged 7 commits into from
Feb 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions etc/redux-toolkit.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,32 +102,27 @@ export function createAction<P = void, T extends string = string>(type: T): Payl
export function createAction<PA extends PrepareAction<any>, T extends string = string>(type: T, prepareAction: PA): PayloadActionCreator<ReturnType<PA>['payload'], T, PA>;

// @alpha (undocumented)
export function createAsyncThunk<ActionType extends string, Returned, ActionParams = void, TA extends AsyncThunksArgs<any, any, any> = AsyncThunksArgs<unknown, unknown, Dispatch>>(type: ActionType, payloadCreator: (args: ActionParams, thunkArgs: TA) => Promise<Returned> | Returned): ((args: ActionParams) => (dispatch: TA["dispatch"], getState: TA["getState"], extra: TA["extra"]) => Promise<import("./createAction").PayloadAction<Returned, string, {
export function createAsyncThunk<ActionType extends string, Returned, ActionParams = void, TA extends AsyncThunksArgs<any, any, any> = AsyncThunksArgs<unknown, unknown, Dispatch>>(type: ActionType, payloadCreator: (args: ActionParams, thunkArgs: TA) => Promise<Returned> | Returned): ((args: ActionParams) => ((dispatch: TA["dispatch"], getState: TA["getState"], extra: TA["extra"]) => Promise<import("./createAction").PayloadAction<Returned, string, {
args: ActionParams;
requestId: string;
}, never> | import("./createAction").PayloadAction<undefined, string, {
args: ActionParams;
requestId: string;
}, Error>>) & {
}, any>>) & {
abort: (reason?: string) => void;
}) & {
pending: import("./createAction").ActionCreatorWithPreparedPayload<[string, ActionParams], undefined, string, never, {
args: ActionParams;
requestId: string;
}>;
rejected: import("./createAction").ActionCreatorWithPreparedPayload<[Error, string, ActionParams], undefined, string, Error, {
rejected: import("./createAction").ActionCreatorWithPreparedPayload<[Error, string, ActionParams], undefined, string, any, {
args: ActionParams;
requestId: string;
}>;
fulfilled: import("./createAction").ActionCreatorWithPreparedPayload<[Returned, string, ActionParams], Returned, string, never, {
args: ActionParams;
requestId: string;
}>;
unwrapResult: (returned: import("./createAction").PayloadAction<Returned, string, {
args: ActionParams;
requestId: string;
}, never> | import("./createAction").PayloadAction<undefined, string, {
args: ActionParams;
requestId: string;
}, Error>) => Returned;
};

// @alpha (undocumented)
Expand Down Expand Up @@ -273,6 +268,13 @@ export type SliceCaseReducers<State> = {

export { ThunkAction }

// @alpha (undocumented)
export function unwrapResult<T>(returned: {
error: any;
} | {
payload: NonNullable<T>;
}): NonNullable<T>;

// @alpha (undocumented)
export type Update<T> = UpdateStr<T> | UpdateNum<T>;

Expand Down
115 changes: 114 additions & 1 deletion src/createAsyncThunk.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { createAsyncThunk, miniSerializeError } from './createAsyncThunk'
import {
createAsyncThunk,
miniSerializeError,
unwrapResult
} from './createAsyncThunk'
import { configureStore } from './configureStore'
import { AnyAction } from 'redux'

describe('createAsyncThunk', () => {
it('creates the action types', () => {
Expand Down Expand Up @@ -104,3 +109,111 @@ describe('createAsyncThunk', () => {
expect(errorAction.meta.args).toBe(args)
})
})

describe('createAsyncThunk with abortController', () => {
const asyncThunk = createAsyncThunk('test', function abortablePayloadCreator(
_: any,
{ 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(new DOMException('Was aborted while running', 'AbortError'))
})
setTimeout(resolve, 100)
})
})

let store = configureStore({
reducer(store: AnyAction[] = []) {
return store
}
})

beforeEach(() => {
store = configureStore({
reducer(store: AnyAction[] = [], action) {
return [...store, action]
}
})
})

test('normal usage', async () => {
await store.dispatch(asyncThunk({}))
expect(store.getState()).toEqual([
expect.any(Object),
expect.objectContaining({ type: 'test/pending' }),
expect.objectContaining({ type: 'test/fulfilled' })
])
})

test('abort after dispatch', async () => {
const thunkAction = asyncThunk({})
const promise = store.dispatch(thunkAction)
thunkAction.abort('AbortReason')
Copy link
Collaborator

@markerikson markerikson Feb 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usage here seems unlikely to occur in a real app. Most code isn't going to reference the thunkAction value at all, as it's going to be dispatch(asyncThunk()).

I realize that the abort handling needs to be on a per-call basis, so I'm not sure what the right answer is here.

I guess the user could explicitly split it into a couple steps if they know ahead of time they might need to cancel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think cancellation is not the most common thing per se, as for many people ignoring earlier running requests might be enough - but if needed this api provides all tools to create a simple reusable abstraction:

function useAbortingApi(asyncApiAction, args) {
  const abortRef = useRef();
  const dispatch = useDispatch();
  return () => {
    const thunkAction = asyncApiAction(args);
    if (abortRef.current) abortRef.current();
    abortRef.current = thunkAction.abort;
    dispatch(thunkAction);
  }
}

function MyComponent(){
  const clickHandler = useAbortingApi(api.fetch, {});
  return <button onClick={clickHandler}>...</button>;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I could try to put an abort method on the promise returned by the thunk... Might even be pretty simple.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That eliminated the "aborted before dispatch" case and allowed for a lot of simplifications.
I hadn't even considered that 🤦‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the diff is ruined now sigh

const result = await promise
const expectedAbortedAction = {
type: 'test/aborted',
error: {
message: 'AbortReason',
name: 'AbortError'
},
meta: { reason: 'AbortReason' }
}
// abortedAction with reason is dispatched after test/pending is dispatched
expect(store.getState()).toMatchObject([
{},
{ type: 'test/pending' },
expectedAbortedAction
])

// same abortedAction is returned, but with the AbortError from the abortablePayloadCreator
expect(result).toMatchObject({
...expectedAbortedAction,
error: {
message: 'Was aborted while running',
name: 'AbortError'
}
})

// calling unwrapResult on the returned object re-throws the error from the abortablePayloadCreator
expect(() => unwrapResult(result)).toThrowError(
expect.objectContaining({
message: 'Was aborted while running',
name: 'AbortError'
})
)
})

test('abort before dispatch', async () => {
const thunkAction = asyncThunk({})
thunkAction.abort('AbortReason')
const result = await store.dispatch(thunkAction)

const expectedAbortedAction = {
type: 'test/aborted',
error: {
message: 'AbortReason',
name: 'AbortError'
},
meta: { reason: 'AbortReason' }
}
// abortedAction with reason is dispatched without test/pending being dispatched
expect(store.getState()).toMatchObject([{}, expectedAbortedAction])

// same abortedAction is returned
expect(result).toMatchObject(expectedAbortedAction)

// unwrapResult throws AbortedError from returned action
expect(() => unwrapResult(result)).toThrowError(
expect.objectContaining(expectedAbortedAction.error)
)
})
})
94 changes: 76 additions & 18 deletions src/createAsyncThunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ import { Dispatch } from 'redux'
import nanoid from 'nanoid'
import { createAction } from './createAction'

type Await<P> = P extends PromiseLike<infer T> ? T : P

type AsyncThunksArgs<S, E, D extends Dispatch = Dispatch> = {
dispatch: D
getState: S
extra: E
requestId: string
signal: AbortSignal
}

interface SimpleError {
Expand Down Expand Up @@ -84,24 +83,67 @@ export function createAsyncThunk<
}
)

const aborted = createAction(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need a separate aborted action type?

Could we use rejected, and have aborted: true as part of meta?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be an option, too. I'm fine either way - your call :)

type + '/aborted',
(requestId: string, args: ActionParams, abortError: DOMException) => {
return {
payload: undefined,
meta: { args, requestId, reason: abortError.message },
error: miniSerializeError(abortError)
}
}
)

const rejected = createAction(
type + '/rejected',
(error: Error, requestId: string, args: ActionParams) => {
return {
payload: undefined,
error,
error: miniSerializeError(error),
meta: { args, requestId }
}
}
)

function actionCreator(args: ActionParams) {
return async (
const abortController = new AbortController()

let dispatchAbort:
| undefined
| ((reason: string) => ReturnType<typeof aborted>)

let abortReason: string

function abort(reason: string = 'Aborted.') {
abortController.abort()
if (dispatchAbort) {
dispatchAbort(reason)
} else {
abortReason = reason
}
}

async function thunkAction(
dispatch: TA['dispatch'],
getState: TA['getState'],
extra: TA['extra']
) => {
) {
const requestId = nanoid()
let abortAction: ReturnType<typeof aborted> | undefined
dispatchAbort = reason => {
abortAction = aborted(
requestId,
args,
new DOMException(reason, 'AbortError')
)
dispatch(abortAction)
return abortAction
}
// if the thunkAction.abort() method has been called before the thunkAction was dispatched,
// just dispatch an aborted-action and never start with the thunk
if (abortController.signal.aborted) {
return dispatchAbort(abortReason)
}

let finalAction: ReturnType<typeof fulfilled | typeof rejected>
try {
Expand All @@ -112,14 +154,26 @@ export function createAsyncThunk<
dispatch,
getState,
extra,
requestId
requestId,
signal: abortController.signal
} as TA),
requestId,
args
)
} catch (err) {
const serializedError = miniSerializeError(err)
finalAction = rejected(serializedError, requestId, args)
if (
err instanceof DOMException &&
err.name === 'AbortError' &&
abortAction
) {
// abortAction has already been dispatched, no further action should be dispatched
// by this thunk.
// return a copy of the dispatched abortAction, but attach the AbortError to it.
return Object.assign({}, abortAction, {
error: miniSerializeError(err)
})
}
finalAction = rejected(err, requestId, args)
}

// We dispatch "success" _after_ the catch, to avoid having any errors
Expand All @@ -129,21 +183,25 @@ export function createAsyncThunk<
dispatch(finalAction)
return finalAction
}
}

function unwrapResult(
returned: Await<ReturnType<ReturnType<typeof actionCreator>>>
) {
if (rejected.match(returned)) {
throw returned.error
}
return returned.payload
return Object.assign(thunkAction, { abort })
}

return Object.assign(actionCreator, {
pending,
rejected,
fulfilled,
unwrapResult
fulfilled
})
}

/**
* @alpha
*/
export function unwrapResult<T>(
returned: { error: any } | { payload: NonNullable<T> }
): NonNullable<T> {
if ('error' in returned) {
throw returned.error
}
return returned.payload
}
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,4 @@ export {
Comparer
} from './entities/models'

export { createAsyncThunk } from './createAsyncThunk'
export { createAsyncThunk, unwrapResult } from './createAsyncThunk'
3 changes: 2 additions & 1 deletion type-tests/files/createAsyncThunk.typetest.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createAsyncThunk, Dispatch, createReducer, AnyAction } from 'src'
import { ThunkDispatch } from 'redux-thunk'
import { promises } from 'fs'
import { unwrapResult } from 'src/createAsyncThunk'

function expectType<T>(t: T) {
return t
Expand Down Expand Up @@ -44,7 +45,7 @@ function fn() {}
}

promise
.then(async.unwrapResult)
.then(unwrapResult)
.then(result => {
expectType<number>(result)
// typings:expect-error
Expand Down