redux-utilities/reduce-reducers

Incorrect TypeScript declaration

vhenzl opened this issue · 7 comments

TypeScript type declaration created in #27 is incorrect. It doesn't allow combining of multiple reducers without an initial state.

Both examples below are taken from tests (with some modifications).

const initialState = {A: 0, B: 0};
type State = typeof initialState;

// this one is OK
const reducer1 = reduceReducer(initialState, (state: State, action: Action) => ({
  ...state,
  A: state.A + 1
}));

// this one fails because first argument is expected to be initial state
const reducer2 = reduceReducer(
  (state: State, action: Action) => ({...state, A: state.A + 2}),
  (state: State, action: Action) => ({...state, B: state.B * 2}),
);

The second example fails with the following error:

Argument of type '(state: { A: number; B: number; }, action: Action) => { B: number; A: number; }' is not assignable to parameter of type 'Reducer<(state: { A: number; B: number; }, action: Action) => { A: number; B: number; }>'.
Types of parameters 'state' and 'state' are incompatible.
Type '(state: { A: number; B: number; }, action: Action) => { A: number; B: number; }' is missing the following properties from type '{ A: number; B: number; }': A, B
Link to TypeScript Playground

https://www.typescriptlang.org/play/index.html#src=%2F%2F%20type%20declarations%2C%20copy%20from%20https%3A%2F%2Fgithub.com%2Fredux-utilities%2Freduce-reducers%2Fblob%2Fmaster%2Findex.d.ts%0D%0Atype%20Action%20%3D%20%7B%0D%0A%20%20type%3A%20string%3B%0D%0A%7D%3B%0D%0A%0D%0Atype%20Reducer%3CS%3E%20%3D%20(state%3A%20S%2C%20action%3A%20Action)%20%3D%3E%20S%3B%0D%0A%0D%0Adeclare%20function%20reduceReducer%3CS%3E(%0D%0A%20%20initialState%3A%20S%2C%0D%0A%20%20...reducers%3A%20Array%3CReducer%3CS%3E%3E%0D%0A)%3A%20Reducer%3CS%3E%3B%0D%0A%0D%0A%2F%2F%20reduce-reducers%20use%2C%20modified%20examples%20from%20https%3A%2F%2Fgithub.com%2Fredux-utilities%2Freduce-reducers%2Fblob%2Fmaster%2Ftest%2Findex.test.js%0D%0Aconst%20initialState%20%3D%20%7BA%3A%200%2C%20B%3A%200%7D%3B%0D%0Atype%20State%20%3D%20typeof%20initialState%3B%0D%0A%0D%0A%2F%2F%20this%20one%20is%20OK%0D%0Aconst%20reducer1%20%3D%20reduceReducer(initialState%2C%20(state%3A%20State%2C%20action%3A%20Action)%20%3D%3E%20(%7B%0D%0A%20%20...state%2C%0D%0A%20%20A%3A%20state.A%20%2B%201%0D%0A%7D))%3B%0D%0A%0D%0A%2F%2F%20this%20one%20fails%20because%20first%20argument%20is%20expected%20to%20be%20initial%20state%0D%0Aconst%20reducer2%20%3D%20reduceReducer(%0D%0A%20%20(state%3A%20State%2C%20action%3A%20Action)%20%3D%3E%20(%7B...state%2C%20A%3A%20state.A%20%2B%202%7D)%2C%0D%0A%20%20(state%3A%20State%2C%20action%3A%20Action)%20%3D%3E%20(%7B...state%2C%20B%3A%20state.B%20*%202%7D)%2C%0D%0A)%3B%0D%0A%0D%0A%2F%2F%20above%20error%3A%0D%0A%2F%2F%20Argument%20of%20type%20'(state%3A%20%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D%2C%20action%3A%20Action)%20%3D%3E%20%7B%20B%3A%20number%3B%20A%3A%20number%3B%20%7D'%20is%20not%20assignable%20to%20parameter%20of%20type%20'Reducer%3C(state%3A%20%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D%2C%20action%3A%20Action)%20%3D%3E%20%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D%3E'.%0D%0A%2F%2F%20Types%20of%20parameters%20'state'%20and%20'state'%20are%20incompatible.%0D%0A%2F%2F%20Type%20'(state%3A%20%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D%2C%20action%3A%20Action)%20%3D%3E%20%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D'%20is%20missing%20the%20following%20properties%20from%20type%20'%7B%20A%3A%20number%3B%20B%3A%20number%3B%20%7D'%3A%20A%2C%20B%0D%0A

I'm facing a similar issue. Looking at the description here: https://redux.js.org/recipes/structuring-reducers/beyond-combinereducers I got the idea that as long as the 1st reducer gives me the inital State I should be fine.

Should I refactor my code to have the initial state set in the reduceReducer or is this a bug?

Doing something like this could work?:

import { Reducer } from 'redux';

export default function reduceReducer<S>(
  initialState: S | null,
  ...reducers: Array<Reducer<S>>
): Reducer<S>;

Even with that fix, I still have to define a lot of stuff as State | undefined.

const crossSliceReducer = (state: State | undefined)=> {
  return state;
};

const combinedReducers = combineReducers<State>({
  mouseCords: mouseCords,
  character: character,
  map: map,
  render: render
});

const gameApp = reduceReducer<State | undefined>(null, combinedReducers, crossSliceReducer);

Or have the crossSliceReducer as "Reducer"

const crossSliceReducer: Reducer = (state: State) => {
  return state;
};

const combinedReducers = combineReducers<State>({
  mouseCords: mouseCords,
  character: character,
  map: map,
  render: render
});

const hexgameApp = reduceReducer(null, combinedReducers, crossSliceReducer);

export default hexgameApp;

My TS is not that strong so I might be missing something tho.

@vhenzl You could bypass this atm by doing:

const reducer2 = reduceReducer(
  null as unknown as State,
  (state: State, action: Action) => ({...state, A: state.A + 2}),
  (state: State, action: Action) => ({...state, B: state.B * 2}),
);

You would be passing null as the first argument, but telling TS that it's actually a State

Hi @tuuling, thanks for trying to be helpful. Your workaround above would work but that would require me to modify tens of files in my project.

The way to go until types are fixed here is to write your own declaration in your project and overwrite typings from this package. You could create, for example, reduce-reducers.d.ts with content like this:

declare module 'reduce-reducers' {
  import { Reducer } from "redux";

  function reduceReducers<S>(initialState: S | null, ...reducers: Reducer<S>[]): Reducer<S>;
  function reduceReducers<S>(...reducers: Reducer<S>[]): Reducer<S>;

  export default reduceReducers;
}

Things to note:

  1. It has to utilise functions overloading to support both cases w/o initialState.
  2. The example assumes only standard reducer signature, i.e. (State, Action) => State, and doesn't support extra arguments in reducers.
  3. Action is of type AnyAction. Probably, it should have a generic type variable for action (reduceReducers<S, A>(...)). And it might require using infer as in combineReducers. But that's currently beyond of my TS knowledge.

Ping to @RumyantsevMichael who created the type declarations in #27.

Hi all. It seems it's my fault. I'll try to figure out in a few days.

@vhenzl Right. About point nr1 Did not notice that you could call it without initialState all together.
About points 2 and 3 I can't comment as I have not used or needed to use those.

Fixed in v1.0.2.

Thanks everyone 👏