reduxjs / redux-toolkit

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

.addCase for multiple actions #429

Closed lucassus closed 4 years ago

lucassus commented 4 years ago

redux-actions has a very nice combineActions helper https://redux-actions.js.org/api/combineactions which allows to reduce multiple distinct actions with the same reducer. It would be nice to have something similar in redux-tookit, so we could easily boil down this:

builder
    .addCase(createTodoSuccess, (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))
    .addCase(updateTodoSuccess, (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

to this:

builder
    .addCases([createTodoSuccess, updateTodoSuccess], (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

WDYT?

phryneas commented 4 years ago

What would the typings imply if createTodoSuccess and updateTodoSuccess had different payloads? This will lead to all kinds of problems down the road.

Honestly, I'm not really a fan. If you really wanted to do this, you could just define your case reducer as a function in outer scope and call addCase with that function multiple times.

markerikson commented 4 years ago

Yeah, it's not an unreasonable suggestion, but I'm inclined to skip it for now. The typings would likely get too weird, and you can handle them by reusing the same reducer.

kirkobyte commented 4 years ago

The typings would likely get too weird

Is that from an internal implementation perspective or for consumers? If it's for consumers, typesafe-actions addresses it by making the action type provided to the reducer a union of the possible actions. In cases where the payload type is incompatible, you can type guard by checking the action type. Personally it feels tidier than splitting out the reducer function and needing to explicitly type it.

phryneas commented 4 years ago

It would massively bloat up the implementation for very little gain. Doing it for the builder notation of extraReducers wouldn't even be so bad by itself. But then people would infer that something similar would have to exist for the builder notation of createReducer. Subsequently for the reducers option of createSlice.

And if you take a look at the typing implementation of createSlice, that one is already very complicated. It actually took months of user feedback until we had every edge case in there.

Adding a builder notation type thing on top of it would at least double that. Or even be impossible. Because in that case, we'd need the return value of that builder chain to evaluate the actions that were added along the way. And we have already encountered countless situations where this leads to circular type references, which is usually solved by having the builder callback return void, which is not possible here.

That cascade of unpleasantries aside: Why not just re-use the same reducer twice?

{
  extraReducers: builder => {
    function sharedReducer(state: State, action: PayloadAction<SharedPayload>){ /*...*/ }

    builder.addCase(actionA, sharedReducer).addCase(actionB, sharedReducer);
  }
}
SaeedEsk commented 3 years ago

if you have used createAsyncThunk check your naming, You may forgot to change the asyncFunction naming convention

ex: `export const checkShopName = createAsyncThunk('shop/check-name', async (data, { rejectWithValue }) => { try { const payload = await ShopService.checkShopName(data); return payload } catch (error) { return rejectWithValue(error.response.data) } })

export const createShop = createAsyncThunk('shop/check-name', async (data, { rejectWithValue }) => { try { const payload = await ShopService.createShop(data); return payload } catch (error) { return rejectWithValue(error.response.data) } })`

in this situation you may get this error

phryneas commented 3 years ago

Update: For anyone reading this a year later: This is actually possible today using the builder notation by combining addMatcher and isAnyOf

builder
    .addMatcher(isAnyOf (createTodoSuccess, updateTodoSuccess), (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

@SaeidEsk I'm a bit confused. Nobody in this whole issue was talking about having any kind of error. This was a feature request.

xmd5a commented 3 years ago

Update: For anyone reading this a year later: This is actually possible today using the builder notation by combining addMatcher and isAnyOf

builder
    .addMatcher(isAnyOf (createTodoSuccess, updateTodoSuccess), (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

@SaeidEsk I'm a bit confused. Nobody in this whole issue was talking about having any kind of error. This was a feature request.

I was looking today for a solution of this problem and this answer is pretty neat - thanks for sharing! 🔥

elpatronaco commented 3 years ago

Update: For anyone reading this a year later: This is actually possible today using the builder notation by combining addMatcher and isAnyOf

builder
    .addMatcher(isAnyOf (createTodoSuccess, updateTodoSuccess), (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

@SaeidEsk I'm a bit confused. Nobody in this whole issue was talking about having any kind of error. This was a feature request.

This is the best implementation so far, we can also play with how many actions have to match to run the matcher. Thank you very much

borisilic-ap commented 3 years ago
builder
    .addMatcher(isAnyOf (createTodoSuccess, updateTodoSuccess), (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

Don't suppose there's a way to do this with PURGE from redux-persist? The isAnyOf complains that PURGE isn't a Matcher.

markerikson commented 3 years ago

@borisilic-ap yes, a "matcher" is just a predicate function that takes an action and returns a boolean, so you'd need to do something like:

const isPersistAction = (action) => action.type === PURGE

See https://redux-toolkit.js.org/api/matching-utilities

suraj946 commented 1 year ago

Update: For anyone reading this a year later: This is actually possible today using the builder notation by combining addMatcher and isAnyOf

builder
    .addMatcher(isAnyOf (createTodoSuccess, updateTodoSuccess), (state, { payload: todo }) => ({
      ...state,
      [todo.id]: todo
    }))

@SaeidEsk I'm a bit confused. Nobody in this whole issue was talking about having any kind of error. This was a feature request.

.addMatcher(isAnyOf("signupRequest", "userRequest"),(state)=>{
     state.loading = true;
})
.addMatcher(isAnyOf("signupSuccess", "userSuccess"),(state, action)=>{
       state.loading = false;
     state.user = action.payload;
    state.isAuthenticated = true;
 })
 .addMatcher(isAnyOf("signupFail", "userFail"),(state, action)=>{
    state.loading = false;
   state.error = action.payload;
   state.isAuthenticated = false;
})

when using the addCase then it is normal but using addMatcher why its setting the user property when its a failure

phryneas commented 1 year ago

@suraj946 isAnyOf is not meant for strings to be passed in - just for type guard functions or action creators.

n-ii-ma commented 1 year ago

@borisilic-ap yes, a "matcher" is just a predicate function that takes an action and returns a boolean, so you'd need to do something like:

const isPersistAction = (action) => action.type === PURGE

See https://redux-toolkit.js.org/api/matching-utilities

How would this work in isAnyOf? Because adding this to it causes this type error:

Argument of type '(action: AnyAction) => boolean' is not assignable to parameter of type 'Matcher<any>'.
  Type '(action: AnyAction) => boolean' is not assignable to type 'TypeGuard<any>'.
    Signature '(action: AnyAction): boolean' must be a type predicate.

I'm trying to add a Matcher to check if the dispatched action is either logout or PURGE (redux-persist) so that I can reset the slice.

const logout = createAction("auth/logout");
const isPersistAction = (action: AnyAction) => action.type === PURGE;

const authSlice = createSlice({
  name: "auth",
  initialState,
  reducers: {},
  extraReducers: (builder) => {
    builder
      .addMatcher(
        isAnyOf(
          authApi.endpoints.verifyOTP.matchFulfilled,
          profileApi.endpoints.updateProfile.matchFulfilled
        ),
        (state, action) => {
          state.user = action.payload.user;
          state.token = action.payload.token;
        }
      )
      // Purge redux-persist state
      .addMatcher(isAnyOf(logout, isPersistAction), () => initialState);
  },
});
EskiMojo14 commented 1 year ago

isAnyOf requires the function to be a type guard if you're using typescript: const isPersistAction = (action: AnyAction): action is Action<typeof PURGE> => action.type === PURGE;