Add runtime deprecation warning for object case reducer syntax in createSlice.extraReducers and createReducer

[markerikson]

We'd like to remove the object syntax for declaring case reducers in createReducer and createSlice in RTK 2.0. The object form was a neat trick when RTK was getting started, and admittedly played nice with action creators having an implicit toString() that returned the action type. But, the builder syntax is basically the same number of lines of code, and also provides much better type safety with TS.

In other words, this:

const todoAdded = createAction('todos/todoAdded');

createReducer(initialState, {
  [todoAdded]: (state, action) => {}
})

createSlice({
  name,
  initialState,
  reducers: {},
  extraReducers: {
    [todoAdded]: (state, action) => {}
  }
})

should be migrated to:

createReducer(initialState, builder => {
  builder.addCase(todoAdded, (state, action) => {})
})

createSlice({
  name,
  initialState,
  reducers: {},
  extraReducers: builder => {
    builder.addCase(todoAdded, (state, action) => {})
  }
})

We should set up a one-warning-per-app load deprecation notice, the way React does:

https://github.com/facebook/react/blob/d5f1b067c8bbb826b823d0354a28ba31078b70c0/packages/react/src/ReactContext.js#L44-L65

[phryneas]

Adding to that: the builder notation is also a lot less problematic with circular references.

[markerikson]

Also: it would be really neat if we could write a codemod to help with this migration!

[SilencerWeb]

Hey @markerikson from Twitter!

Will look in the evening what I can do in order to help to write a codemod for an easier migration

I'll keep you updated!

[martinerko]

hey guys, I had a quick look and I was able to implement both codemods. Added also some tests. check it out.
In next PR I can add some codemod runner and README.

[ryota-murakami]

I agree with this idea.

Personally I don't use that object syntax, I'm not sure how common pattern that is though.

[einarq]

Doesn't this mean we have then again forced to manually create action creators using "createAction", instead of getting them automatically exported from slice.actions? Seems like something that is going in the wrong direction wrt some if the initial motivation behind RTK, which was to get rid of the dreaded "boilerplate".
Or is this only for the "extraReducers"? Maybe I'm missing something?

[phryneas]

You are missing something. This is about extraReducers only, which has two different notations. Both don't create actions.

Don't do this any more:

  extraReducers: {
    [incrementBy]: (state, action) => {
      return state + action.payload
    },
    'some/other/action': (state, action) => {},
  },

Do this instead:

  extraReducers: (builder) => {
    builder
      .addCase(incrementBy, (state, action) => {
        // action is inferred correctly here if using TS
      })
      // You can chain calls, or have separate `builder.addCase()` lines each time
      .addCase(decrement, (state, action) => {})
      // You can match a range of action types
      .addMatcher(
        isRejectedAction,
        // `action` will be inferred as a RejectedAction due to isRejectedAction being defined as a type guard
        (state, action) => {}
      )
      // and provide a default case if no other handlers matched
      .addDefaultCase((state, action) => {})
  },

[einarq]

Right, I came back here to write that I had now gathered that from reading the other "2.0 planning" thread :)

It wasn't completely obvious that this only involved createReducer and extraReducers from reading this sentence:
"We'd like to remove the object syntax for declaring case reducers in createReducer and createSlice in RTK 2.0."

Anyway, I can see the motivation for doing this, no objections here :)

[markerikson]

Added this in #2591 !

Also we should make sure to get the codemods out alongside 1.9 so folks can change their usages sooner: #2592

[cdpark0530]

In version 1.9.0, I can't find a way to type PayloadAction in builder pattern.

// createSlice.d.ts
export interface CreateSliceOptions<State = any, CR extends SliceCaseReducers<State> = SliceCaseReducers<State>, Name extends string = string> {
  ...
  extraReducers?: CaseReducers<NoInfer<State>, any> | ((builder: ActionReducerMapBuilder<NoInfer<State>>) => void);
}

// mapBuilders.d.ts
export interface ActionReducerMapBuilder<State> {
  ...
  // Where can I specify `PayloadAction`?
  addCase<ActionCreator extends TypedActionCreator<string>>(actionCreator: ActionCreator, reducer: CaseReducer<State, ReturnType<ActionCreator>>): ActionReducerMapBuilder<State>;
  addCase<Type extends string, A extends Action<Type>>(type: Type, reducer: CaseReducer<State, A>): ActionReducerMapBuilder<State>;
}

You might consider to make such a way in version 2

[markerikson]

@cdpark0530 : you don't. PayloadAction should be used when typing functions in the reducers field. When using builder.addCase(), you should be passing in action creators that are already typed, and RTK is designed to have TS infer the type of action from there:

import { todoAdded } from "../someOtherFeature/someOtherFeatureSlice";

const newSlice = createSlice({
  name: "whatever",
  initialState,
  reducers: {
    // normal reducers here
  }
  extraReducers: (builder) => {
    builder.addCase(todoAdded, (state, action) => {
      // TS will know that `action` is a `PayloadAction<Todo>` because of type inference
    })
  }
})

[AngelHadzhiev]

Adding to that: the builder notation is also a lot less problematic with circular references.

Hey,
I saw you comment under this issue. I am not very experienced with Redux and I want to ask how does the old approach lead to circular references? Why does the new apprach does not do the same?

[markerikson]

@AngelHadzhiev : we changed createSlice to do a lazy evaluation and creation of the slice reducer on demand. That delays the read of imported variables just long enough for them to finish initializing before they're referenced. See #1686