feat(RouterStore): Added serializer for router state snapshot

[brandonroberts]

This adds a serializer that can be customized for returning the router state snapshot. By default, the entire RouterStateSnapshot is returned. Documentation has been updated with example usage.

import { StoreModule } from '@ngrx/store';
import {
  StoreRouterConnectingModule,
  routerReducer,
  RouterStateSerializer
} from '@ngrx/router-store';

export interface RouterStateUrl {
  url: string;
}

export class CustomSerializer implements RouterStateSerializer<RouterStateUrl> {
  serialize(routerState: RouterStateSnapshot): RouterStateUrl {
    const url = routerState ? routerState.url : '';

    // Only return an object including the URL
    // instead of the entire snapshot
    return { url };
  }
}

@NgModule({
  imports: [
    StoreModule.forRoot({ routerReducer: routerReducer }),
    RouterModule.forRoot([
      // routes
    ]),
    StoreRouterConnectingModule
  ],
  providers: [
    { provide: RouterStateSerializer, useClass: CustomSerializer }
  ]
})
export class AppModule { }

Closes #97, #104, #237

[kylecordes]

From a quick read of the code, it looks like this keeps the default behavior of sticking the unserializable router state in the State, but makes it possible for an application to avoid that by providing something custom, for example by copying code from the example application.

My way-out-here-in-userland perspective is that it would be more helpful if the default serializer did the "Right Thing", so that most applications get a serializable State without effort. I dream of a future where Store runs in a "frozen, enforced serializability" mode by default, and requires a StoreModule.forRoot(mapOfReducers, { footGun: true}) invocation to turn that enforcement off.

[brandonroberts]

@kylecordes I agree that the default should be a slimmed down version of the router state snapshot. It would be a breaking change to the current functionality though, so we'll keep it as is, deprecate it and change the default in the next version. Me and @MikeRyanDev have talked about have a config option out of the box to freeze the state, but it was more of an opt-in approach. We'll revisit that soon for the next version, as we'll need a replacement for ngrx-store-freeze if its no longer maintained.

[vsavkin]

why |null?

[vsavkin]

Maybe we guard against nulls outside? Cause I think it should never be called with null.

[vsavkin]

I'm not sure if partial is actually useful there as it it is not recursive and as such you either have to provide the whole root node or none.

[vsavkin]

basically the following will fail:

const r: RouterStateSnapshotType;
r.root.params // won't compile

[vsavkin]

Maybe we can parameterize RouterNavigationAction instead?

[vsavkin]

Not a big fan of the Type suffix.

Maybe SerializedRouterStateSnapshot?

[brandonroberts]

Good suggestion

[coveralls]

Coverage increased (+0.08%) to 91.287% when pulling 9a4a26b on router-snapshot-serializer into 7d23fdb on master.

[coveralls]

Coverage increased (+0.08%) to 91.287% when pulling 858de40 on router-snapshot-serializer into 7d23fdb on master.

[brandonroberts]

@vsavkin this one is ready for another review

[vsavkin]

LGTM

[strizzaflex]

@brandonroberts Thanks for the workaround.

1. When can we expect the next major version with the actual fix? (Ballpark)
2. Having store freeze built in would be a big thumbs up from me. Our team is new to redux and one of the biggest pitfalls up front was mutating state.

Again thanks for the fix we are back up and running with the nightly build.