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

fix: type mismatch when upgrade to ts2.6 #903

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

sandangel
Copy link
Contributor

Refs: #709

@coveralls
Copy link

coveralls commented Mar 11, 2018

Coverage Status

Coverage remained the same at 93.533% when pulling 56d75a9 on sandangel:fix-ts2.6 into 839b42a on ngrx:master.

@brandonroberts
Copy link
Member

This is to support strict: true correct? We're already building against 2.6 in the repo. We're not going to change Action to any as we lose the type safety.

@sandangel
Copy link
Contributor Author

sandangel commented Mar 12, 2018

yes, this is for supporting strict: true.
could you suggest a better fix for map type between reducer and action type when there are multiple features in ActionReducerMap ?
currently this won't work:

import * as fromAuth from './auth.reducer';
import * as fromToken from './token.reducer';

export interface AuthFeatureState {
  authState: fromAuth.AuthState;
  tokenState: fromToken.TokenState;
}

export const reducers1: ActionReducerMap<AuthFeatureState> = {
  authState: fromAuth.authReducer, // <- error type string is not assignable to string literal
  tokenState: fromToken.tokenReducer // <- error
};

export const reducers2: ActionReducerMap<AuthFeatureState, AuthActions | TokenActions> = {
  authState: fromAuth.authReducer, // <- error type of TokenActions is not assignable to AuthActions
  tokenState: fromToken.tokenReducer // <- error
};

user have manually cast ActionReducerMap<AuthFeatureState, any> for ts to compile.

@sandangel
Copy link
Contributor Author

if you not agree to change the default type variable in ActionReducerMap, I can revert it, but do you consider the fix for routerStore signature ?

@brandonroberts
Copy link
Member

The routerStore signature fix is ok. I'd rather use ActionReducerMap<AuthFeatureState, any> for now but those changes don't impact this repo as strict: true isn't enabled for the example-app

@sandangel
Copy link
Contributor Author

hi @brandonroberts , I have revert the ActionReducerMap default type, please check.

@brandonroberts brandonroberts merged commit f17a032 into ngrx:master Mar 13, 2018
@sandangel sandangel deleted the fix-ts2.6 branch March 13, 2018 02:01
sandangel added a commit to sandangel/platform that referenced this pull request Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants