reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.74k stars 1.17k forks source link

Pass abort reason of thunk action to AbortController #2395

Open xbaun opened 2 years ago

xbaun commented 2 years ago

Hi,

I noticed that the abort reason of a thunk action is not getting passed down to the AbortController in line 578: https://github.com/reduxjs/redux-toolkit/blob/64a30d83384d77bcbc59231fa32aa2f1acd67020/packages/toolkit/src/createAsyncThunk.ts#L575-L580

It would be useful to get the abort reason directly in the thunk action, so the thunk can cleanup its states.

thx

phryneas commented 2 years ago

AbortController only added support for a reason recently and it is spotty across different platforms. If you can come up with a good overview on where it is supported and where it might cause problems passing it in we could get to an informed decision, but I don't have the time to do that research atm.

xbaun commented 2 years ago

There is a MDN overview for reason where it is supported https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason#browser_compatibility - looks pretty good.

It's also supported on node with version v16 and above: https://nodejs.org/docs/latest-v16.x/api/globals.html#abortcontrollerabortreason

Are there other platforms do you mean?

FaberVitale commented 2 years ago

AbortController only added support for a reason recently and it is spotty across different platforms. If you can come up with a good overview on where it is supported and where it might cause problems passing it in we could get to an informed decision, but I don't have the time to do that research atm.

In listenerMiddleware code, reason is patched if it is not avaible at runtime: https://github.com/reduxjs/redux-toolkit/blob/2b80d8be3785f6e74fcbee88c58fb6d27c330a57/packages/toolkit/src/listenerMiddleware/utils.ts#L43

We could reuse abortControllerWithReason in asyncThunks