Replacing 'routerReducer' with a configurable option

[phillipzada]

See #410

• Creation of a forRoot method to allow for configuration of StoreRouterConnectingModule
• Replacement of 'routerReducer' static state key with a configurable value

[coveralls]

Changes Unknown when pulling 4583010 on PhillipZada:master into ** on ngrx:master**.

[brandonroberts]

This can be added to the imports below from ../src/index

[brandonroberts]

This would be a breaking change for existing users unless you add default providers to to NgModule metadata

[brandonroberts]

This won't work in AoT because the actual value for useValue property won't get resolved. You'll need to have two tokens, one for internal use that you register using useValue and another you register with useFactory that uses the first token as a dependency. Then you'll inject the public token in the constructor.

[brandonroberts]

Initialize this value in the factory function used to register the token

[phillipzada]

Hey @brandonroberts just wanted to get your thoughts my my previous comments.

[brandonroberts]

Hey @phillipzada. If you're talking about jasmine-marbles, I just responded to your comments.

[phillipzada]

@brandonroberts sorry mate, I was referring the comments to your comments to the code changes above.

Hey Brandon,

Would this work, you do you still prefer an internal and external token?

export function createDefaultRouterConfig(config: StoreRouterConfig = {}): StoreRouterConfig {
  return {
    stateKey: 'routerReducer',
    ...config
  }
}



@NgModule({
  providers: [
    { provide: RouterStateSerializer, useClass: DefaultRouterStateSerializer },
    { provide: ROUTER_CONFIG, useFactory: createDefaultRouterConfig }
  ],
})
export class StoreRouterConnectingModule {
  static forRoot(config?: StoreRouterConfig): ModuleWithProviders;
  static forRoot(config: StoreRouterConfig = {}): ModuleWithProviders {
    return {
      ngModule: StoreRouterConnectingModule,
      providers: [{ provide: ROUTER_CONFIG, useFactory: createDefaultRouterConfig, deps: [config] }],
    };
  }

  private routerState: RouterStateSnapshot;
  private storeState: any;
  private lastRoutesRecognized: RoutesRecognized;

  private dispatchTriggeredByRouter: boolean = false; // used only in dev mode in combination with routerReducer
  private navigationTriggeredByDispatch: boolean = false; // used only in dev mode in combination with routerReducer

  private stateKey: string;

  constructor(
    private store: Store<any>,
    private router: Router,
    private serializer: RouterStateSerializer<RouterStateSnapshot>,
    @Inject(ROUTER_CONFIG) private config: StoreRouterConfig
  ) {
    const routerConfig = createDefaultRouterConfig(config);
    this.stateKey = (routerConfig.stateKey as string);

[brandonroberts]

@phillipzada changes look good for the most part. You can't use config as a dependency without it being a token though. And then you won't need to call createDefaultRouterConfig in the constructor.

[phillipzada]

Thanks @brandonroberts, appreciate the assist, will make the changes this week.

[phillipzada]

Hey @brandonroberts, Changes have been made.

[coveralls]

Changes Unknown when pulling cbbe801 on phillipzada:master into ** on ngrx:master**.

[brandonroberts]

Thanks @phillipzada. One small thing. Change @ngrx/router to @ngrx/router-store in the InjectionToken strings. Everything else looks good.

[brandonroberts]

The providers in the forRoot method should be in the NgModule metadata also to prevent a breaking change for existing users.

[brandonroberts]

@phillipzada Can you fix the merge conflicts and failing tests? I'm ready to land this one.

[coveralls]

Coverage decreased (-0.05%) to 92.494% when pulling dae762f on phillipzada:master into 565389a on ngrx:master.

[coveralls]

Coverage decreased (-0.05%) to 92.494% when pulling 3e5ff01 on phillipzada:master into 4db7d61 on ngrx:master.

[brandonroberts]

@phillipzada can you remove the yarn.lock file from the changes? There shouldn't be any dependency changes.

[brandonroberts]

@phillipzada sorry, I wasn't clear. Can you remove the yarn.lock changes? The yarn.lock file in master should remain unchanged

[phillipzada]

So make a copy of the current version and put it back in?

[coveralls]

Coverage decreased (-0.05%) to 92.494% when pulling d04b3f6 on phillipzada:master into 4db7d61 on ngrx:master.