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

@ngrx/router-store: StoreRouterConfig should use generics to better support custom serializer #1457

Closed
homj opened this issue Dec 8, 2018 · 0 comments · Fixed by #1460
Closed

Comments

@homj
Copy link

homj commented Dec 8, 2018

Replacing the built in serializer with a custom one results in a type warning, when using a selector in the StoreRouterConfig:

interface MyRouterState extends BaseRouterStoreState {
    customParam: any;
}

class MyRouterStateSerializer extends RouterStateSerializer<MyRouterState> {
    // ...
}

const selectRouter: Selector<any, RouterReducerState<MyRouterState>> = // ...;

// in the Module:
StoreRouterConnectingModule.forRoot({
    stateKey: selectRouter,
    serializer: MyRouterStateSerializer
})

results in the warning:

TS2322:
Type 'MemoizedSelector<object, RouterReducerState<MyRouterState>>' is not assignable to type 'StateKeyOrSelector'.
Type 'MemoizedSelector<object, RouterReducerState<MyRouterState>>' is not assignable to type 'Selector<any, RouterReducerState<SerializedRouterStateSnapshot>>'.
Type 'RouterReducerState<MyRouterState>' is not assignable to type 'RouterReducerState<SerializedRouterStateSnapshot>'.
Types of property 'state' are incompatible.
Type 'MyRouterState' is not assignable to type 'SerializedRouterStateSnapshot'.
Property 'root' is missing in type 'MyRouterState'.

The proposed change would be to use generics on StateKeyOrSelector and StoreRouterConfig:

export declare type StateKeyOrSelector<ROUTER_STATE extends BaseRouterStoreState = SerializedRouterStateSnapshot> = string | Selector<any, RouterReducerState<ROUTER_STATE>>;

export interface StoreRouterConfig<ROUTER_STATE extends BaseRouterStoreState = SerializedRouterStateSnapshot> {
  stateKey?: StateKeyOrSelector<ROUTER_STATE>;
  serializer?: new (...args: any[]) => RouterStateSerializer<ROUTER_STATE>;
  navigationActionTiming?: NavigationActionTiming;
}

Describe any alternatives/workarounds you're currently using

Currently, the only way to prevent this issue is not typing the selector correctly (which is bad imo), or suppressing the warning with @ts-ignore (which is also not great, since it makes me wonder whether the root property of SerializedRouterStateSnapshot is necessary or not)

Other information:

Demo on StackBlitz

If accepted, I would be willing to submit a PR for this feature

[ ] Yes (Assistance is provided if you need help submitting a pull request)
[x] No

brandonroberts pushed a commit that referenced this issue Dec 12, 2018
* feat(RouterStore): make the router store key selector generic
* docs: add note that router stateKey can also be a selector

Closes #1457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants