fix(RouterStore): type mismatch #794
Conversation
| @@ -96,7 +96,7 @@ export type RouterReducerState<T = RouterStateSnapshot> = { | |||
| }; | |||
|
|
|||
| export function routerReducer<T = RouterStateSnapshot>( | |||
| state: RouterReducerState<T>, | |||
| state: RouterReducerState<T> | undefined, | |||
There was a problem hiding this comment.
Had to use undefined because the signature for an ActionReducer is (state: T | undefined, action: V)
example-app/app/reducers/index.ts
Outdated
| @@ -39,14 +40,14 @@ export interface State { | |||
| * These reducer functions are called with each dispatched action | |||
| * and the current or initial state and return a new immutable state. | |||
| */ | |||
| export const reducers: ActionReducerMap<State> = { | |||
| export const reducers: ActionReducerMap<State, any> = { | |||
There was a problem hiding this comment.
Not quite happy with any but I think this is the only way to do it.
Feedback welcome!
There was a problem hiding this comment.
Although this will fix router store but users still have to manually add ‘any’ by themself to by pass typescript check in their store. we need a better approach
There was a problem hiding this comment.
What about the last commit? I think we're getting closer...
| export class RouterNavigationAction<T = RouterStateSnapshot> implements Action { | ||
| readonly type = ROUTER_NAVIGATION; | ||
| constructor(public payload: RouterNavigationPayload<T>) {} | ||
| } |
There was a problem hiding this comment.
after typescript 2.8 landed which support conditional type, I think ngrx team will stick with interface based Action
There was a problem hiding this comment.
Oh I did not know about this!
But since we're still in 2.7 this is the only way to do it.
There was a problem hiding this comment.
If we need to use classes here, make them abstract
|
Does this PR only cover support for router-store with TS 2.6 and Angular 5.2? |
| @@ -8,9 +8,9 @@ const initialState: State = { | |||
| showSidenav: false, | |||
| }; | |||
|
|
|||
| export function reducer( | |||
There was a problem hiding this comment.
Why is this change needed for router-store?
There was a problem hiding this comment.
Otherwise the example app will have the following error when we create the ActionReducerMap:
Type '{ layout: (state: State | undefined, action: LayoutActions) => State; router: <V extends RouterAc...' is not assignable to type 'ActionReducerMap<State, Action>'.
| export class RouterNavigationAction<T = RouterStateSnapshot> implements Action { | ||
| readonly type = ROUTER_NAVIGATION; | ||
| constructor(public payload: RouterNavigationPayload<T>) {} | ||
| } |
There was a problem hiding this comment.
If we need to use classes here, make them abstract
|
I'm thinking TS 2.4.2 and up but I'm not 100% sure. |
|
So, if I'm not mistaken it should work with TS 2.4.2 and up.
|
Had to create
Actions instead of types to makeActionReducerMaphappy.Closes #709