Fix bug in docs for createAsyncThunk example

[dlwalsh]

Resolves #416

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 32f7219:

Sandbox	Source
awesome-resonance-qc5zy	Configuration
dazzling-bouman-wl89o	Configuration
confident-lovelace-0nef6	Configuration

[markerikson]

Hmm. Now that I look at this, I feel like this needs a bit more thought put into it.

How would you distinguish between "this specific thunk has dispatched the pending action and is now doing the payload work", and "some other request is already in flight"?

@msutkowski , @phryneas - any thoughts on this?

[dlwalsh]

I guess you'd need to store the requestId and check against that.

[msutkowski]

@markerikson With the way createAsyncThunk currently works, the user would have to make a simple wrapper that tracked the promise resolution state and use it like:
const myTrackedPromise = promiseState(dispatch(myLongRunningAsyncThunk()))

If this seems like something we'd want to handle internally, a quick and low-effort solution would be to expose a getStatus or isProcessing function on the promise and have it return the internal state.

const everlongPromise = dispatch(myLongRunningThunk());
// ... do a lot of processing and such?
if (everlongPromise.getStatus() === 'processing') {
  // we know we're processing still...
}

or

const everlongPromise = dispatch(myLongRunningThunk());
// ... do a lot of processing and such?
if (everlongPromise.isProcessing()) {
  // we know we're processing still...
}

Would someone actually want more of the internals like requestId, or just to know if we're still in the middle of processing?

[msutkowski]

Here is a basic sample of that idea: #418. Switched the naming from my example above to match the standardized naming.

You know what, I just realized I misunderstood the original question. I'll follow back up in the morning 🤦‍♂️

[phryneas]

I also think we should just go with the requestId.
Also, I don't know if the payloadCreator itself should generally check that it should be running. It might make sense in this case, but that is a very edgy case and I wouldn't really use that as an example then.
Generally: It has been started and not been cancelled, so it runs. I would assume that it's only the job of the reducer to ignore (or not) actions sent from the thunk.

So this would be my attempt:

import { createSlice, createAsyncThunk, SerializedError } from "@reduxjs/toolkit";

function fetchById(id: string) {
  return new Promise<{ data: string }>(resolve => setTimeout(resolve, 1000, { data: "test" }));
}

const fetchUserById = createAsyncThunk("users/fetchByIdStatus", async (userId: string) => {
  if (getState().slice.requestId !== requestId){
    // if you really do want to do something like this, I'd do it this way
    // but generally, the reducer can handle this on it's own
    return
  }
  const response = await fetchById(userId);
  return response.data;
});

const usersSlice = createSlice({
  name: "users",
  initialState: {
    currentRequestId: undefined as undefined|string,
    entities: [] as string[],
    loading: "idle",
    error: null as null | SerializedError
  },
  reducers: {},
  extraReducers: builder =>
    builder
      .addCase(fetchUserById.pending, (state, action) => {
        if (state.loading === "idle") {
          state.loading = "pending";
          state.currentRequestId = action.meta.requestId
        }
      })
      .addCase(fetchUserById.fulfilled, (state, action) => {
        if (state.currentRequestId === action.meta.requestId) {
          state.loading = "idle";
          state.entities.push(action.payload);
        }
      })
      .addCase(fetchUserById.rejected, (state, action) => {
        if (state.currentRequestId === action.meta.requestId) {
          state.loading = "idle";
          state.error = action.error;
        }
      })
});

[dlwalsh]

Updated PR to implement the pattern above.

[dlwalsh]

Thanks for the feedback @phryneas. I've updated the code snippet accordingly.

You've also clarified my own confusion around the createAsyncThunk API. I suspect the fact that the fulfilled action still fires on early exit will be surprising to some. It was to me.

[phryneas]

Thanks for the feedback @phryneas. I've updated the code snippet accordingly.

You've also clarified my own confusion around the createAsyncThunk API. I suspect the fact that the fulfilled action still fires on early exit will be surprising to some. It was to me.

A return statement is a return statement and will trigger a fulfilled action. From the outside we cannot observe why something returned, and undefined might be a valid return value.
Maybe throwing something in that example might be better - then it'd be a rejected action.
But something will definitely happen - it also does when cancelling the thunk by calling .abort() on it - it's always guaranteed to have a pending action followed by either an fulfilled or rejected action.