Skip to content

Commit

Permalink
feat(effects): throw error when forRoot() is used more than once
Browse files Browse the repository at this point in the history
  • Loading branch information
timdeschryver authored and brandonroberts committed Sep 7, 2019
1 parent 4304865 commit b46748c
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 4 deletions.
27 changes: 27 additions & 0 deletions modules/effects/spec/integration.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { NgModuleFactoryLoader, NgModule } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import {
RouterTestingModule,
SpyNgModuleFactoryLoader,
} from '@angular/router/testing';
import { Router } from '@angular/router';
import { Store, Action } from '@ngrx/store';
import {
EffectsModule,
Expand All @@ -20,6 +26,7 @@ describe('NgRx Effects Integration spec', () => {
RootEffectWithInitActionWithPayload,
]),
EffectsModule.forFeature([FeatEffectWithInitAction]),
RouterTestingModule.withRoutes([]),
],
providers: [
{
Expand Down Expand Up @@ -73,6 +80,21 @@ describe('NgRx Effects Integration spec', () => {
]);
});

it('throws if forRoot() is used more than once', (done: DoneFn) => {
let router: Router = TestBed.get(Router);
const loader: SpyNgModuleFactoryLoader = TestBed.get(NgModuleFactoryLoader);

loader.stubbedModules = { feature: FeatModuleWithForRoot };
router.resetConfig([{ path: 'feature-path', loadChildren: 'feature' }]);

router.navigateByUrl('/feature-path').catch((err: TypeError) => {
expect(err.message).toBe(
'EffectsModule.forRoot() called twice. Feature modules should use EffectsModule.forFeature() instead.'
);
done();
});
});

class RootEffectWithInitAction implements OnInitEffects {
ngrxOnInitEffects(): Action {
return { type: '[RootEffectWithInitAction]: INIT' };
Expand Down Expand Up @@ -110,4 +132,9 @@ describe('NgRx Effects Integration spec', () => {

constructor(private effectIdentifier: string) {}
}

@NgModule({
imports: [EffectsModule.forRoot([])],
})
class FeatModuleWithForRoot {}
});
24 changes: 22 additions & 2 deletions modules/effects/src/effects_module.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { NgModule, ModuleWithProviders, Type } from '@angular/core';
import {
NgModule,
ModuleWithProviders,
Type,
Optional,
SkipSelf,
} from '@angular/core';
import { EffectSources } from './effect_sources';
import { Actions } from './actions';
import { ROOT_EFFECTS, FEATURE_EFFECTS } from './tokens';
import { ROOT_EFFECTS, FEATURE_EFFECTS, _ROOT_EFFECTS_GUARD } from './tokens';
import { EffectsFeatureModule } from './effects_feature_module';
import { EffectsRootModule } from './effects_root_module';
import { EffectsRunner } from './effects_runner';
Expand Down Expand Up @@ -31,6 +37,11 @@ export class EffectsModule {
return {
ngModule: EffectsRootModule,
providers: [
{
provide: _ROOT_EFFECTS_GUARD,
useFactory: _provideForRootGuard,
deps: [[EffectsRunner, new Optional(), new SkipSelf()]],
},
EffectsRunner,
EffectSources,
Actions,
Expand All @@ -48,3 +59,12 @@ export class EffectsModule {
export function createSourceInstances(...instances: any[]) {
return instances;
}

export function _provideForRootGuard(runner: EffectsRunner): any {

This comment has been minimized.

Copy link
@edlin

edlin Jan 29, 2020

Our application uses EffectsModule.forRoot a couple places because we want to make sure that effects is initialized EffectsModule.forFeature. This worked when we had a single js bundle, but we recently started to lazy load and this check is breaking things. What was the rational behind this change? @timdeschryver

This comment has been minimized.

Copy link
@kylecannon

kylecannon Mar 5, 2020

Contributor

I just added a PR that will hopefully address this: #2426

if (runner) {
throw new TypeError(
`EffectsModule.forRoot() called twice. Feature modules should use EffectsModule.forFeature() instead.`
);
}
return 'guarded';
}
7 changes: 5 additions & 2 deletions modules/effects/src/effects_root_module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from '@ngrx/store';
import { EffectsRunner } from './effects_runner';
import { EffectSources } from './effect_sources';
import { ROOT_EFFECTS } from './tokens';
import { ROOT_EFFECTS, _ROOT_EFFECTS_GUARD } from './tokens';

export const ROOT_EFFECTS_INIT = '@ngrx/effects/init';

Expand All @@ -19,7 +19,10 @@ export class EffectsRootModule {
store: Store<any>,
@Inject(ROOT_EFFECTS) rootEffects: any[],
@Optional() storeRootModule: StoreRootModule,
@Optional() storeFeatureModule: StoreFeatureModule
@Optional() storeFeatureModule: StoreFeatureModule,
@Optional()
@Inject(_ROOT_EFFECTS_GUARD)
guard: any
) {
runner.start();

Expand Down
3 changes: 3 additions & 0 deletions modules/effects/src/tokens.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { InjectionToken, Type } from '@angular/core';

export const _ROOT_EFFECTS_GUARD = new InjectionToken<void>(
'@ngrx/effects Internal Root Guard'
);
export const IMMEDIATE_EFFECTS = new InjectionToken<any[]>(
'ngrx/effects: Immediate Effects'
);
Expand Down

0 comments on commit b46748c

Please sign in to comment.