From 8dc2b119fb3c4fcb43964a08515820952b836463 Mon Sep 17 00:00:00 2001 From: Daniel Wiehl Date: Sun, 5 Aug 2018 15:35:51 +0200 Subject: [PATCH] fix(router): mount correct component if router outlet was not instantiated and if using a route reuse strategy (#25313) (#25314) This unsets 'attachRef' on outlet context if no route is to be reused in route activation. Closes #25313 PR Close #25314 --- packages/platform-browser/testing/BUILD.bazel | 1 + .../platform-browser/testing/rollup.config.js | 1 + .../platform-browser/testing/src/matchers.ts | 33 +++++++- packages/router/src/router.ts | 1 + packages/router/test/integration.spec.ts | 82 ++++++++++++++++++- 5 files changed, 113 insertions(+), 5 deletions(-) diff --git a/packages/platform-browser/testing/BUILD.bazel b/packages/platform-browser/testing/BUILD.bazel index 7f94b76b21..97fb49e925 100644 --- a/packages/platform-browser/testing/BUILD.bazel +++ b/packages/platform-browser/testing/BUILD.bazel @@ -10,6 +10,7 @@ ng_module( module_name = "@angular/platform-browser/testing", deps = [ "//packages/core", + "//packages/core/testing", "//packages/platform-browser", "@rxjs", ], diff --git a/packages/platform-browser/testing/rollup.config.js b/packages/platform-browser/testing/rollup.config.js index 9409d2131c..d2424ff7f5 100644 --- a/packages/platform-browser/testing/rollup.config.js +++ b/packages/platform-browser/testing/rollup.config.js @@ -11,6 +11,7 @@ const sourcemaps = require('rollup-plugin-sourcemaps'); const globals = { '@angular/core': 'ng.core', + '@angular/core/testing': 'ng.core.testing', '@angular/common': 'ng.common', '@angular/platform-browser': 'ng.platformBrowser' }; diff --git a/packages/platform-browser/testing/src/matchers.ts b/packages/platform-browser/testing/src/matchers.ts index 3c9ae4ba48..40ea6593f1 100644 --- a/packages/platform-browser/testing/src/matchers.ts +++ b/packages/platform-browser/testing/src/matchers.ts @@ -7,8 +7,9 @@ */ -import {ɵglobal as global} from '@angular/core'; -import {ɵgetDOM as getDOM} from '@angular/platform-browser'; +import {Type, ɵglobal as global} from '@angular/core'; +import {ComponentFixture} from '@angular/core/testing'; +import {By, ɵgetDOM as getDOM} from '@angular/platform-browser'; @@ -79,6 +80,11 @@ export interface NgMatchers extends jasmine.Matchers { */ toContainError(expected: any): boolean; + /** + * Expect a component of the given type to show. + */ + toContainComponent(expectedComponentType: Type, expectationFailOutput?: any): boolean; + /** * Invert the matchers. */ @@ -235,6 +241,29 @@ _global.beforeEach(function() { }; } }; + }, + + toContainComponent: function() { + return { + compare: function(actualFixture: any, expectedComponentType: Type) { + const failOutput = arguments[2]; + const msgFn = (msg: string): string => [msg, failOutput].filter(Boolean).join(', '); + + // verify correct actual type + if (!(actualFixture instanceof ComponentFixture)) { + return { + pass: false, + message: msgFn( + `Expected actual to be of type \'ComponentFixture\' [actual=${actualFixture.constructor.name}]`) + }; + } + + const found = !!actualFixture.debugElement.query(By.directive(expectedComponentType)); + return found ? + {pass: true} : + {pass: false, message: msgFn(`Expected ${expectedComponentType.name} to show`)}; + } + }; } }); }); diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index dfe8e95eae..44ff3a07df 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -1043,6 +1043,7 @@ class ActivateRoutes { const config = parentLoadedConfig(future.snapshot); const cmpFactoryResolver = config ? config.module.componentFactoryResolver : null; + context.attachRef = null; context.route = future; context.resolver = cmpFactoryResolver; if (context.outlet) { diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index c5967d3092..a71cc6c76a 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -8,13 +8,13 @@ import {CommonModule, Location} from '@angular/common'; import {SpyLocation} from '@angular/common/testing'; -import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core'; +import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, OnDestroy, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core'; import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router'; -import {Observable, Observer, of } from 'rxjs'; -import {map} from 'rxjs/operators'; +import {Observable, Observer, Subscription, of } from 'rxjs'; +import {filter, map} from 'rxjs/operators'; import {forEach} from '../src/utils/collection'; import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing'; @@ -4022,6 +4022,82 @@ describe('Integration', () => { const simpleCmp2 = fixture.debugElement.children[1].componentInstance; expect(simpleCmp1).not.toBe(simpleCmp2); }))); + + it('should not mount the component of the previously reused route when the outlet was not instantiated at the time of route activation', + fakeAsync(() => { + @Component({ + selector: 'root-cmp', + template: + '
' + }) + class RootCmpWithCondOutlet implements OnDestroy { + private subscription: Subscription; + public isToolpanelShowing: boolean = false; + + constructor(router: Router) { + this.subscription = + router.events.pipe(filter(event => event instanceof NavigationEnd)) + .subscribe( + () => this.isToolpanelShowing = + !!router.parseUrl(router.url).root.children['toolpanel']); + } + + public ngOnDestroy(): void { this.subscription.unsubscribe(); } + } + + @Component({selector: 'tool-1-cmp', template: 'Tool 1 showing'}) + class Tool1Component { + } + + @Component({selector: 'tool-2-cmp', template: 'Tool 2 showing'}) + class Tool2Component { + } + + @NgModule({ + declarations: [RootCmpWithCondOutlet, Tool1Component, Tool2Component], + imports: [ + CommonModule, + RouterTestingModule.withRoutes([ + {path: 'a', outlet: 'toolpanel', component: Tool1Component}, + {path: 'b', outlet: 'toolpanel', component: Tool2Component}, + ]), + ], + }) + class TestModule { + } + + TestBed.configureTestingModule({imports: [TestModule]}); + + const router: Router = TestBed.get(Router); + router.routeReuseStrategy = new AttachDetachReuseStrategy(); + + const fixture = createRoot(router, RootCmpWithCondOutlet); + + // Activate 'tool-1' + router.navigate([{outlets: {toolpanel: 'a'}}]); + advance(fixture); + expect(fixture).toContainComponent(Tool1Component, '(a)'); + + // Deactivate 'tool-1' + router.navigate([{outlets: {toolpanel: null}}]); + advance(fixture); + expect(fixture).not.toContainComponent(Tool1Component, '(b)'); + + // Activate 'tool-1' + router.navigate([{outlets: {toolpanel: 'a'}}]); + advance(fixture); + expect(fixture).toContainComponent(Tool1Component, '(c)'); + + // Deactivate 'tool-1' + router.navigate([{outlets: {toolpanel: null}}]); + advance(fixture); + expect(fixture).not.toContainComponent(Tool1Component, '(d)'); + + // Activate 'tool-2' + router.navigate([{outlets: {toolpanel: 'b'}}]); + advance(fixture); + expect(fixture).toContainComponent(Tool2Component, '(e)'); + })); }); });