Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Previous implementations of Redux reducers using combineReducers broken in 2.4.1 #16795

Closed
jimsugg opened this issue Jun 28, 2017 · 13 comments
Closed
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@jimsugg
Copy link

jimsugg commented Jun 28, 2017

TypeScript Version: 2.4.1
Code

// From Redux declaration file
interface Action {
  type: any;
}
type Reducer<S> = <A extends Action>(state: S, action: A) => S;
interface ReducersMapObject {
  [key: string]: Reducer<any>;
}
declare function combineReducers<S>(reducers: ReducersMapObject): Reducer<S>;

// User code
interface BsDmBaseAction extends Action {
  type: string;  // override to a string type
  payload: number;
}

interface State1 {
  id: string;
  data: number;
}

function statesById(state: State1[] = [], action: BsDmBaseAction): State1[] {
  switch (action.type) {
    case 'STOP': return [{id: 'status', data: action.payload}];
  }
  return state;
}

function allIds(state: string[] = [], action: BsDmBaseAction): string[] {
  switch (action.type) {
    case 'STOP': return ['status'];
  }
  return state;
}

const bigReducer = combineReducers({
  statesById,
  allIds,
});

Expected behavior:
No errors. This has been working since Typescript 1.8, and works in 2.3.4.

Actual behavior:
Starting with Typescript 2.4.1, the following error is emitted for the combineReducers call:
Error:(40, 36) TS2345:Argument of type '{ statesById: (state: State1[], action: BsDmBaseAction) => State1[]; allIds: (state: string[], ac...' is not assignable to parameter of type 'ReducersMapObject'.
Property 'statesById' is incompatible with index signature.
Type '(state: State1[], action: BsDmBaseAction) => State1[]' is not assignable to type 'Reducer'.
Types of parameters 'action' and 'action' are incompatible.
Type 'A' is not assignable to type 'BsDmBaseAction'.
Type 'Action' is not assignable to type 'BsDmBaseAction'.
Property 'payload' is missing in type 'Action'.

Notes
I do not see any explanation in the various notes on 'breaking changes' to explain this. There are no weak types here, which was involved in the main 'breaking change' description.

@weswigham
Copy link
Member

This would probably be a result of strict contravariance for callback parameters.

@jimsugg
Copy link
Author

jimsugg commented Jun 28, 2017

It may be indeed, although I must admit I still fail to see how it is a 'bug' as described in the paragraph you cite. In any case, it is a pretty significant disaster for those of us using Redux, assuming we want to keep using Typescript also. Thanks for pointing to the paragraph though.

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Jun 28, 2017

@weswigham I think this is a valid TypeScript bug. If I change the reducer to type Reducer<S> = (state: S, action: Action) => S;, it compiles.

@jimsugg
Copy link
Author

jimsugg commented Jun 28, 2017

Fair enough, it does. Let me see if I can extend that approach into our more complex real world scenario. Also, now I'm curious if it has always been the case that your syntax would work. Why was the other more obtuse syntax ('') ever used? Anyway, thanks.

@jimsugg
Copy link
Author

jimsugg commented Jun 28, 2017

Well, that works, but the change has to be in the Redux index.d.ts.

@weswigham
Copy link
Member

There's already an open PR.

@jimsugg
Copy link
Author

jimsugg commented Jun 28, 2017

Yep - found that - I have pulled that down and am testing it. It works well. There's a suggestion for an alternative that I will test today. A big issue is that it's going to be really clunky until they actually get the fix into the Redux distribution. I can get that into my work ok, but rolling it out to my team will be problematic until it's actually part of Redux.

@DonikaV
Copy link

DonikaV commented Jul 4, 2017

@jimsugg
Hey, i tried to use your suggestion but have error in console
Uncaught ReferenceError: combineReducers is not defined

My reducers/index.ts looks like

import { sessionReducer } from './session';
import { routerReducer } from 'react-router-redux';
interface Action {
	type: any;
}
type Reducer<S> = (state: S, action: Action) => S;

interface ReducersMapObject {
	[key: string]: Reducer<any>;
}
declare function combineReducers<S>(reducers: ReducersMapObject): Reducer<S>;

export const reducers =  combineReducers({
	sessionReducer,
        routing: routerReducer,
});

Can you help me? Thanks.

@willisplummer
Copy link

i think this issue can be closed! reduxjs/redux#2467 fixed for me

@mhegazy mhegazy added the External Relates to another program, environment, or user action which we cannot control. label Aug 22, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Sep 6, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed Sep 6, 2017
@lenguyenthanh
Copy link

lenguyenthanh commented Nov 12, 2017

It is broken again with Typescript 2.6.1 and Redux 3.7.2
Here is my code:

import State from './state'
import { defaultState } from './state'
import { combineReducers, Reducer } from 'redux'
import { Dispatch, Store, Action } from 'redux'
import { reducer as count } from '../feature/counter/counterScreen'

export enum keys {
  INCREMENT = 'increment',
  DECREMENT = 'decrement',
}

interface IncrementAction {
  readonly type: keys.INCREMENT
  readonly payload: {
    readonly size: number
  }
}
const incrementAction = (size: number): IncrementAction => ({
  type: keys.INCREMENT,
  payload: {
    size: size,
  },
})

interface DecrementAction {
  readonly type: keys.DECREMENT
  readonly payload: {
    readonly size: number
  }
}
const decrementAction = (size: number): DecrementAction => ({
  type: keys.DECREMENT,
  payload: {
    size: size,
  },
})

export type ActionTypes = IncrementAction | DecrementAction

/** REDUCER */
export function reducer(state = defaultState.count, action: ActionTypes) {
  switch (action.type) {
    case keys.DECREMENT:
      return state - action.payload.size
    case keys.INCREMENT:
      return state + action.payload.size
    default:
      return state
  }
}

const rootReducers: Reducer<State> = combineReducers({ count })

export default rootReducers

and here is error:

yarn build v0.27.5
$ yarn run clean && yarn run lint && yarn tsc --
src/app/reducers.ts(53,54): error TS2345: Argument of type '{ count: (state: number | undefined, action: ActionTypes) => number; }' is not assignable to parameter of type 'ReducersMapObject'.
  Property 'count' is incompatible with index signature.
    Type '(state: number | undefined, action: ActionTypes) => number' is not assignable to type 'Reducer<any>'.
      Types of parameters 'action' and 'action' are incompatible.
        Type 'AnyAction' is not assignable to type 'ActionTypes'.
          Type 'AnyAction' is not assignable to type 'DecrementAction'.
            Property 'payload' is missing in type 'AnyAction'.

It works well if I change Reducer definition in index.d.ts file:

export type Reducer<S, A extends AnyAction> = (state: S, action: A) => S;

And my reducer:

const rootReducers: Reducer<State, ActionTypes> = combineReducers({ count })

@jhalborg
Copy link

jhalborg commented Nov 22, 2017

Confirmed, TS 2.6.1 and redux 3.7.2 with similar setup as @lenguyenthanh mentions

@lenguyenthanh
Copy link

@jhalborg I created an issue in Redux repository: reduxjs/redux#2709.
So I'm using Redux 3.7.2 with index.d.ts file from Redux@4.0.0-beta.1. It works for me.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

8 participants