Add support for defining thunks in createSlice

[markerikson]

Here's one that's been asked for a lot since we came up with createSlice: adding some kind of syntax for defining thunks directly inside of the createSlice call.

Now that I've actually used createAsyncThunk a few times in the new Redux tutorial I'm writing, I can see that there's still some pain points there:

• hand-declaring the thunk separately from the slice
• having to duplicate the slice name in your action type prefix arg
• having to always define extraReducers and defining the handlers for each result

We're going to ignore the issue of defining loading state and stuff for now, and focus on what a possible syntax for this might look like.

The biggest complicating factor here is the TS usage. createAsyncThunk may require several possible generic args, including {dispatch, state, extra}. We can't know, at the time the slice is defined, what the state type might be.

@phryneas listed several possible syntaxes in chat discussion, along with why they won't work. Pasting that here for reference:

So it seems to work without direct circularity problems. But, we don't really have a way to inject any generics there, so we don't really have any way to specify types for dispatch, extra & getState. So currently we can really only accept any in there :/
Probably the only way around that would be to define generic inferfaces for state/dispatch/extra for the RTK module that could be extended by module augmentation. So,

declare module `@reduxjs/toolkit` {
 interface AppRootState extends ReturnType<typeof store.getState> {}
 // not sure if that would even work
 interface AppDispatch extends typeof store.dispatch 
  interface AppExtra //...
}

Not really a fan of that thought/
Or use some kind of second-level builder that could allow for generics like

createSlice({
  asyncThunks: {
    thunk1: createAsyncThunk => createAsyncThunk</*...*/>(/*...*/),
    thunk2: createAsyncThunk => createAsyncThunk</*...*/>(/*...*/),
  }
})

also, not a fan. Also, it would have to be nested that deep down to still be able to extract the keys to create action creators.

createSlice({
  asyncThunks: buildThunks => buildThunks<ApiSignature>({
    thunk1: {
      payloadCreator(...){},
      fulfilled(state, action) {} 
    },
    thunk2: {
      payloadCreator(...){},
      fulfilled(state, action) {} 
    }
  })
})

probably could work as well, but doesn't look too much better. Also, it might need a double-method call to do partial type currying, so more like

createSlice({
  asyncThunks: buildThunks => buildThunks<ApiSignature>()({
    thunk1: {
      payloadCreator(...){},
      fulfilled(state, action) {} 
    },
    thunk2: {
      payloadCreator(...){},
      fulfilled(state, action) {} 
    }
  })
})

However, I noted that Easy-Peasy uses a thunk() helper, and after a bit more discussion:

On the other hand... we could make it a property of createSlice:

createSlice({
  reducers: {
    test: createSlice.thunk</*...*/>({ /*...*/ }),
    test2: createSlice.thunk</*...*/>({ /*...*/ }),
    test3: createSlice.thunk</*...*/>({ /*...*/ })
  }
})

The generic arguments would only need to be specified if api would be used.
Writing it out:

createSlice({
  reducers: {
    test: createSlice.thunk({ 
      preparePayload: ...,
      fulfilled: ...,
      rejected, ...,
      pending: ....
     })
  }
})

We could do that, but make that createSlice.thunks with an s. But not only RootState, but RootState, DispatchType and Extra. RejectedValue should be able to be inferred.

I think it's something that's worth pursuing.

[msutkowski]

I think this last option looks great syntax-wise. My only concern would be about how easy it would be to have an authSlice where this ends up in extraReducers in many other slices and introduces circular import issues. That being said, I don't think it's too much overhead to say, "hey, if you run into X errors, just move it into a regular createAsyncThunk in a common file". The docs do cover this already, but we'd probably just want to reemphasize it in the createAsyncThunk portion assuming this moves forward.

A question about this syntax: would there be any issue checking for these in addMatchers in the scenario that you wanted to set a boolean based on the pending/rejected of this? Or would this need to be defined here as shown and the 'external' createAsyncThunks could be handled with the matcher? (don't mind me - as long as the described behavior remains the same, this is a non-issue. per the docs: Note that all matching matcher reducers will execute in order, even if a case reducer has already executed.)

I can't really think of any other usage issues at this point.

[phryneas]

So I've created a kinda-somehow created something that almost works, but not quite.

It looks like this:
81c4619#diff-d3f6ba32c4f3b495b630095111a617e9R465-R540

The problem here is that there is no way to have the State type be correctly inferred there, so it has to be manually pre-curried like this:

  const createSliceAsyncThunk = curryWithState<TestState>()

which of course is very suboptimal.

We could get around that, but again: only with a builder notation. And as reducers already takes functions as keys, those could not be defined on the reducers option.

Only feasible way for that would be to add a new key, so we'd end up with something like I initially described:

  createSlice({
    name: 'test',
    initialState: {} as TestState,
    specialReducers: {
      testInfer: create => create.asyncThunk(
        (arg: TestArg, api) => {
          return Promise.resolve({ payload: 'foo' } as TestReturned)
        },
        {
          pendingReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
          },
          fulfilledReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
            expectType<TestReturned>(action.payload)
          },
          rejectedReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
            expectType<SerializedError>(action.error)
          }
        }
      )
    }
  })

(and yeah, the split into payloadCreator as first arg and the rest of the config as a second arg seems to be necessary, unfortunately)

[phryneas]

Or we go for something like

  createSlice({
    name: 'test',
    initialState: {} as TestState,
    reducers: withSpecial => ({
      testInfer: withSpecial.asyncThunk(
        (arg: TestArg, api) => {
          return Promise.resolve({ payload: 'foo' } as TestReturned)
        },
        {
          pendingReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
          },
          fulfilledReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
            expectType<TestReturned>(action.payload)
          },
          rejectedReducer(state, action) {
            expectType<TestState>(state)
            expectType<TestArg>(action.meta.arg)
            expectType<SerializedError>(action.error)
          }
        }
      )
    })
  })

[phryneas]

So purely from a type standpoint it looks like that would be possible:
https://github.com/phryneas/redux-toolkit/blob/1915bd092d0deb7abf358360230d860cd97060e8/type-tests/files/createSlice.typetest.ts#L486

(With some hiccups that I'd still need to solve and serious need for refactoring)

[markerikson]

As much as I want this, doesn't seem like it's going to happen due to TS complexity: #637 .