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

Router Store: Make usage of forRoot required #1662

Closed
brandonroberts opened this issue Mar 29, 2019 · 0 comments · Fixed by #1672
Closed

Router Store: Make usage of forRoot required #1662

brandonroberts opened this issue Mar 29, 2019 · 0 comments · Fixed by #1672

Comments

@brandonroberts
Copy link
Member

brandonroberts commented Mar 29, 2019

You can currently register @ngrx/router-store in two ways.

StoreRouterConnectingModule

and

StoreRouterConnectingModule.forRoot(optionalConfig)

This change would only make the forRoot() option available and removes the default providers from the NgModule metadata below.

https://github.com/ngrx/platform/blob/master/modules/router-store/src/router_store_module.ts#L134

Also replaces usage examples to use forRoot()

https://github.com/ngrx/platform/blob/master/modules/router-store/src/router_store_module.ts#L125

EnricoVogt added a commit to EnricoVogt/platform that referenced this issue Mar 31, 2019
EnricoVogt added a commit to EnricoVogt/platform that referenced this issue Mar 31, 2019
EnricoVogt added a commit to EnricoVogt/platform that referenced this issue Mar 31, 2019
BREAKING CHANGES:

usage of forRoot is now required for StoreRouterConnectionModule

BEFORE:

@NgModule({
  providers: [
    {
      provide: _ROUTER_CONFIG,
      useValue: {},
    },
    {
      provide: ROUTER_CONFIG,
      useFactory: _createRouterConfig,
      deps: [_ROUTER_CONFIG],
    },
    {
      provide: RouterStateSerializer,
      useClass: DefaultRouterStateSerializer,
    },
  ],
})
export class StoreRouterConnectingModule {
  static forRoot<
    T extends BaseRouterStoreState = SerializedRouterStateSnapshot
  >(
    config: StoreRouterConfig<T> = {}
  ): ModuleWithProviders<StoreRouterConnectingModule> {
    return {
      ngModule: StoreRouterConnectingModule,
      providers: [
        { provide: _ROUTER_CONFIG, useValue: config },
        {
          provide: RouterStateSerializer,
          useClass: config.serializer
            ? config.serializer
            : DefaultRouterStateSerializer,
        },
      ],
    };
}

AFTER:

@NgModule()
export class StoreRouterConnectingModule {
  static forRoot<
    T extends BaseRouterStoreState = SerializedRouterStateSnapshot
  >(
    config: StoreRouterConfig<T> = {}
  ): ModuleWithProviders<StoreRouterConnectingModule> {
    return {
      ngModule: StoreRouterConnectingModule,
      providers: [
        { provide: _ROUTER_CONFIG, useValue: config },
        {
          provide: ROUTER_CONFIG,
          useFactory: _createRouterConfig,
          deps: [_ROUTER_CONFIG],
        },
        {
          provide: RouterStateSerializer,
          useClass: config.serializer
            ? config.serializer
            : DefaultRouterStateSerializer,
        },
      ],
    };
  }
EnricoVogt added a commit to EnricoVogt/platform that referenced this issue Apr 1, 2019
BREAKING CHANGES:

usage of forRoot is now required for StoreRouterConnectionModule

BEFORE:

@NgModule({
  imports: [
    StoreRouterConnectingModule
  ]
})
export class AppModule {}

AFTER:

@NgModule({
  imports: [
    StoreRouterConnectingModule.forRoot()
  ]
})
export class AppModule {}
brandonroberts pushed a commit that referenced this issue Apr 1, 2019
Closes #1662 

BREAKING CHANGES:

usage of forRoot is now required for StoreRouterConnectingModule

BEFORE:

```ts
@NgModule({
  imports: [
    StoreRouterConnectingModule
  ]
})
export class AppModule {}
```

AFTER:

```ts
@NgModule({
  imports: [
    StoreRouterConnectingModule.forRoot()
  ]
})
export class AppModule {}
```
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.

1 participant