-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Changes from 1 commit
b6fbefc
074d82e
421a657
c1d83ce
63bfbae
27b0aa1
e43f4da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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', () => { | ||
|
@@ -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') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, I could try to put an There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
@@ -84,24 +83,67 @@ export function createAsyncThunk< | |
} | ||
) | ||
|
||
const aborted = createAction( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we actually need a separate Could we use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
@@ -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 | ||
|
@@ -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 | ||
phryneas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
} | ||
|
||
/** | ||
* @alpha | ||
*/ | ||
export function unwrapResult<T>( | ||
returned: { error: any } | { payload: NonNullable<T> } | ||
): NonNullable<T> { | ||
if ('error' in returned) { | ||
throw returned.error | ||
} | ||
return returned.payload | ||
} |
Uh oh!
There was an error while loading. Please reload this page.