Skip to content

Commit

Permalink
fix: fix check which ensures that Workbench.forRoot() is not used i…
Browse files Browse the repository at this point in the history
…n a lazy context

Issue: `Workbench.forChild()` failed if being used from a lazy loaded module.

fixes #5
  • Loading branch information
danielwiehl committed Dec 11, 2018
1 parent a71ca27 commit ea3a1b0
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 10 deletions.
5 changes: 5 additions & 0 deletions projects/scion/workbench/src/lib/workbench.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export const ROUTER_OUTLET_NAME = new InjectionToken<string>('ROUTER_OUTLET_NAME
*/
export const ROUTE_REUSE_PROVIDER = new InjectionToken<WbRouteReuseProvider>('ROUTE_REUSE_PROVIDER');

/**
* DI injection token to ensure `WorkbenchModule.forRoot()` is not used in a lazy context.
*/
export const WORKBENCH_FORROOT_GUARD = new InjectionToken<void>('WORKBENCH_FORROOT_GUARD');

/**
* Represents the name of the activity router outlet.
*/
Expand Down
24 changes: 14 additions & 10 deletions projects/scion/workbench/src/lib/workbench.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* SPDX-License-Identifier: EPL-2.0
*/

import { ANALYZE_FOR_ENTRY_COMPONENTS, InjectionToken, ModuleWithProviders, NgModule, Optional, SkipSelf } from '@angular/core';
import { ANALYZE_FOR_ENTRY_COMPONENTS, Inject, InjectionToken, ModuleWithProviders, NgModule, Optional, SkipSelf } from '@angular/core';
import { CommonModule } from '@angular/common';

import { WorkbenchComponent } from './workbench.component';
Expand Down Expand Up @@ -49,7 +49,7 @@ import { WbActivityDirective } from './activity-part/wb-activity.directive';
import { MoveDirective } from './move.directive';
import { WorkbenchConfig } from './workbench.config';
import { OverlayTemplateOutletDirective } from './overlay-template-outlet.directive';
import { ROUTE_REUSE_PROVIDER } from './workbench.constants';
import { ROUTE_REUSE_PROVIDER, WORKBENCH_FORROOT_GUARD } from './workbench.constants';
import { NotificationService } from './notification/notification.service';
import { NotificationListComponent } from './notification/notification-list.component';
import { NotificationComponent } from './notification/notification.component';
Expand Down Expand Up @@ -117,10 +117,8 @@ const CONFIG = new InjectionToken<WorkbenchConfig>('WORKBENCH_CONFIG');
})
export class WorkbenchModule {

// Specify services to be instantiated eagerly
constructor(@Optional() @SkipSelf() parentModule: WorkbenchModule,
viewRegistrySynchronizer: ViewRegistrySynchronizer) {
WorkbenchModule.throwIfAlreadyLoaded(parentModule);
// Note: We are injecting ViewRegistrySynchronizer so it gets created eagerly...
constructor(@Optional() @Inject(WORKBENCH_FORROOT_GUARD) guard: any, viewRegistrySynchronizer: ViewRegistrySynchronizer) {
}

/**
Expand Down Expand Up @@ -199,6 +197,11 @@ export class WorkbenchModule {
provide: RouteReuseStrategy,
useClass: WbRouteReuseStrategy,
},
{
provide: WORKBENCH_FORROOT_GUARD,
useFactory: provideForRootGuard,
deps: [[WorkbenchService, new Optional(), new SkipSelf()]]
},
]
};
}
Expand All @@ -212,12 +215,13 @@ export class WorkbenchModule {
providers: [] // do not register any providers in 'forChild' but in 'forRoot' instead
};
}
}

private static throwIfAlreadyLoaded(parentModule: any): void {
if (parentModule) {
throw new Error('WorkbenchModule.forRoot() called twice. Lazy loaded modules should use WorkbenchModule.forChild() instead.');
}
export function provideForRootGuard(workbench: WorkbenchService): any {
if (workbench) {
throw new Error('WorkbenchModule.forRoot() called twice. Lazy loaded modules should use WorkbenchModule.forChild() instead.');
}
return 'guarded';
}

export function newConfig(config: WorkbenchConfig): WorkbenchConfig {
Expand Down
110 changes: 110 additions & 0 deletions projects/scion/workbench/src/lib/workbench.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Copyright (c) 2018 Swiss Federal Railways
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/

import { async, fakeAsync, inject, TestBed } from '@angular/core/testing';
import { Component, NgModule, NgModuleFactoryLoader } from '@angular/core';
import { RouterTestingModule, SpyNgModuleFactoryLoader } from '@angular/router/testing';
import { Router, RouterModule } from '@angular/router';
import { WorkbenchModule } from './workbench.module';
import { CommonModule } from '@angular/common';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { advance } from './routing/testing.spec';
import { WorkbenchRouter } from './routing/workbench-router.service';

describe('Workbench', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [AppTestModule]
});

TestBed.get(Router).initialNavigation();
}));

it('throws an error when forRoot() is used in a lazy context', fakeAsync(inject([WorkbenchRouter, NgModuleFactoryLoader], (wbRouter: WorkbenchRouter, loader: SpyNgModuleFactoryLoader) => {
loader.stubbedModules = {
'./feature-a/feature-a.module#FeatureAModule': FeatureAModule,
'./feature-b/feature-b.module#FeatureBModule': FeatureBModule,
};

const fixture = TestBed.createComponent(AppComponent);
advance(fixture);

let recordedError: Error = null;

// Load lazy module with WorkbenchModule.forRoot() import
wbRouter.navigate(['wrong-import'], {blankViewPartRef: 'viewpart.1'}).catch(err => recordedError = err);
advance(fixture);
expect(recordedError.message).toEqual('WorkbenchModule.forRoot() called twice. Lazy loaded modules should use WorkbenchModule.forChild() instead.');

// Load lazy module with WorkbenchModule.forChild() import
recordedError = null;
wbRouter.navigate(['correct-import'], {blankViewPartRef: 'viewpart.1'}).catch(err => recordedError = err);
advance(fixture);
expect(recordedError).toBeNull();
})));
});

/****************************************************************************************************
* Definition of App Test Module *
****************************************************************************************************/
@Component({template: '<wb-workbench style="position: relative; width: 100%; height: 500px"></wb-workbench>'})
class AppComponent {
}

@NgModule({
imports: [
WorkbenchModule.forRoot(),
NoopAnimationsModule,
RouterTestingModule.withRoutes([
{path: 'wrong-import', loadChildren: './feature-a/feature-a.module#FeatureAModule'},
{path: 'correct-import', loadChildren: './feature-b/feature-b.module#FeatureBModule'},
]),
],
declarations: [AppComponent]
})
class AppTestModule {
}

/****************************************************************************************************
* Definition of Feature Module A *
****************************************************************************************************/
@Component({template: 'Feature Module A'})
class ModuleAComponent {
}

@NgModule({
imports: [
CommonModule,
RouterModule.forChild([{path: '', component: ModuleAComponent}]),
WorkbenchModule.forRoot() // simulate wrong import
],
declarations: [ModuleAComponent]
})
export class FeatureAModule {
}

/****************************************************************************************************
* Definition of Feature Module B *
****************************************************************************************************/
@Component({template: 'Feature Module B'})
class ModuleBComponent {
}

@NgModule({
imports: [
CommonModule,
RouterModule.forChild([{path: '', component: ModuleBComponent}]),
WorkbenchModule.forChild() // simulate correct import
],
declarations: [ModuleBComponent]
})
export class FeatureBModule {
}

0 comments on commit ea3a1b0

Please sign in to comment.