Skip to content

Commit

Permalink
fix(router): lazy loaded modules without RouterModule.forChild() won'…
Browse files Browse the repository at this point in the history
…t cause an infinite loop (#36605)

When loading a module that doesn't provide `RouterModule.forChild()` preloader will get stuck
in an infinite loop and throw `ERROR Error: Maximum call stack size exceeded.`
The issue is that child module's `Injector` will look to its parent `Injector` when it doesn't
find any `ROUTES` so it will return routes for it's parent module instead. This will load the
child again that returns its parent's routes and so on.

Closes #29164

PR Close #36605
  • Loading branch information
martinsik authored and AndrewKushnir committed Jan 15, 2021
1 parent 9c595b0 commit 6675b6f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/router/src/router_config_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Compiler, InjectionToken, Injector, NgModuleFactory, NgModuleFactoryLoader} from '@angular/core';
import {Compiler, InjectFlags, InjectionToken, Injector, NgModuleFactory, NgModuleFactoryLoader} from '@angular/core';
import {from, Observable, of} from 'rxjs';
import {map, mergeMap} from 'rxjs/operators';

Expand Down Expand Up @@ -41,8 +41,13 @@ export class RouterConfigLoader {

const module = factory.create(parentInjector);

// When loading a module that doesn't provide `RouterModule.forChild()` preloader will get
// stuck in an infinite loop. The child module's Injector will look to its parent `Injector`
// when it doesn't find any ROUTES so it will return routes for it's parent module instead.
return new LoadedRouterConfig(
flatten(module.injector.get(ROUTES)).map(standardizeConfig), module);
flatten(module.injector.get(ROUTES, undefined, InjectFlags.Self | InjectFlags.Optional))
.map(standardizeConfig),
module);
}));
}

Expand Down
17 changes: 17 additions & 0 deletions packages/router/test/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5291,6 +5291,10 @@ describe('Integration', () => {
class LoadedModule1 {
}

@NgModule({})
class EmptyModule {
}

beforeEach(() => {
log.length = 0;
TestBed.configureTestingModule({
Expand Down Expand Up @@ -5355,6 +5359,19 @@ describe('Integration', () => {
expect(firstConfig).toBeUndefined();
expect(log.length).toBe(0);
}));

it('should allow navigation to modules with no routes', fakeAsync(() => {
(TestBed.inject(NgModuleFactoryLoader) as SpyNgModuleFactoryLoader).stubbedModules = {
empty: EmptyModule,
};
const router = TestBed.inject(Router);
const fixture = createRoot(router, RootCmp);

router.resetConfig([{path: 'lazy', loadChildren: 'empty'}]);

router.navigateByUrl('/lazy');
advance(fixture);
}));
});

describe('custom url handling strategies', () => {
Expand Down
31 changes: 31 additions & 0 deletions packages/router/test/router_preloader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,35 @@ describe('RouterPreloader', () => {
expect(c[0]._loadedConfig!.routes[0].component).toBe(configs[0].component);
})));
});

describe(
'should work with lazy loaded modules that don\'t provide RouterModule.forChild()', () => {
@NgModule({
declarations: [LazyLoadedCmp],
imports: [RouterModule.forChild([{path: 'LoadedModule1', component: LazyLoadedCmp}])]
})
class LoadedModule {
}

@NgModule({})
class EmptyModule {
}

beforeEach(() => {
TestBed.configureTestingModule({
imports: [RouterTestingModule.withRoutes(
[{path: 'lazyEmptyModule', loadChildren: 'expected2'}])],
providers: [{provide: PreloadingStrategy, useExisting: PreloadAllModules}]
});
});

it('should work',
fakeAsync(inject(
[NgModuleFactoryLoader, RouterPreloader],
(loader: SpyNgModuleFactoryLoader, preloader: RouterPreloader) => {
loader.stubbedModules = {expected2: EmptyModule};

preloader.preload().subscribe();
})));
});
});

0 comments on commit 6675b6f

Please sign in to comment.