From c805082648b3067a7b0f3d21a1d1ea0edf77e4df Mon Sep 17 00:00:00 2001 From: vikerman Date: Tue, 16 May 2017 15:14:55 -0700 Subject: [PATCH] fix(platform-server): wait for async app initializers to complete before removing server side styles (#16712) This fixes a flicker when transitioning from server rendered page to client rendered page in lazy loaded routes by waiting for the lazy loaded route to finish loading, assuming initialNavigation on the route is set to 'enabled'. Fixes #15716 --- packages/core/src/application_init.ts | 37 ++++++++++++++++--- packages/core/src/application_ref.ts | 1 + packages/core/test/application_init_spec.ts | 26 +++++++++++-- packages/core/test/application_ref_spec.ts | 16 +++----- packages/core/testing/src/test_bed.ts | 5 ++- .../src/browser/server-transition.ts | 25 +++++++------ 6 files changed, 79 insertions(+), 31 deletions(-) diff --git a/packages/core/src/application_init.ts b/packages/core/src/application_init.ts index e73c649f5c..fdc1966a00 100644 --- a/packages/core/src/application_init.ts +++ b/packages/core/src/application_init.ts @@ -24,23 +24,48 @@ export const APP_INITIALIZER = new InjectionToken void>>('Applicatio */ @Injectable() export class ApplicationInitStatus { + private resolve: Function; + private reject: Function; + private initialized = false; private _donePromise: Promise; private _done = false; - constructor(@Inject(APP_INITIALIZER) @Optional() appInits: (() => any)[]) { + constructor(@Inject(APP_INITIALIZER) @Optional() private appInits: (() => any)[]) { + this._donePromise = new Promise((res, rej) => { + this.resolve = res; + this.reject = rej; + }); + } + + /** @internal */ + runInitializers() { + if (this.initialized) { + return; + } + const asyncInitPromises: Promise[] = []; - if (appInits) { - for (let i = 0; i < appInits.length; i++) { - const initResult = appInits[i](); + + const complete = + () => { + this._done = true; + this.resolve(); + } + + if (this.appInits) { + for (let i = 0; i < this.appInits.length; i++) { + const initResult = this.appInits[i](); if (isPromise(initResult)) { asyncInitPromises.push(initResult); } } } - this._donePromise = Promise.all(asyncInitPromises).then(() => { this._done = true; }); + + Promise.all(asyncInitPromises).then(() => { complete(); }).catch(e => { this.reject(e); }); + if (asyncInitPromises.length === 0) { - this._done = true; + complete(); } + this.initialized = true; } get done(): boolean { return this._done; } diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index ceeb4ee893..9cf06ea52e 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -302,6 +302,7 @@ export class PlatformRef_ extends PlatformRef { ngZone !.onError.subscribe({next: (error: any) => { exceptionHandler.handleError(error); }}); return _callAndReportToErrorHandler(exceptionHandler, () => { const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus); + initStatus.runInitializers(); return initStatus.donePromise.then(() => { this._moduleDoBootstrap(moduleRef); return moduleRef; diff --git a/packages/core/test/application_init_spec.ts b/packages/core/test/application_init_spec.ts index 179bb5613d..ae0bd543c4 100644 --- a/packages/core/test/application_init_spec.ts +++ b/packages/core/test/application_init_spec.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {Injector} from '@angular/core'; import {APP_INITIALIZER, ApplicationInitStatus} from '../src/application_init'; import {TestBed, async, inject} from '../testing'; @@ -14,11 +15,13 @@ export function main() { it('should return true for `done`', async(inject([ApplicationInitStatus], (status: ApplicationInitStatus) => { + status.runInitializers(); expect(status.done).toBe(true); }))); it('should return a promise that resolves immediately for `donePromise`', async(inject([ApplicationInitStatus], (status: ApplicationInitStatus) => { + status.runInitializers(); status.donePromise.then(() => { expect(status.done).toBe(true); }); }))); }); @@ -26,15 +29,32 @@ export function main() { describe('with async initializers', () => { let resolve: (result: any) => void; let promise: Promise; + let completerResolver = false; beforeEach(() => { + let initializerFactory = (injector: Injector) => { + return () => { + const initStatus = injector.get(ApplicationInitStatus); + initStatus.donePromise.then(() => { expect(completerResolver).toBe(true); }); + } + }; promise = new Promise((res) => { resolve = res; }); - TestBed.configureTestingModule( - {providers: [{provide: APP_INITIALIZER, multi: true, useValue: () => promise}]}); + TestBed.configureTestingModule({ + providers: [ + {provide: APP_INITIALIZER, multi: true, useValue: () => promise}, + { + provide: APP_INITIALIZER, + multi: true, + useFactory: initializerFactory, + deps: [Injector] + }, + ] + }); }); it('should update the status once all async initializers are done', async(inject([ApplicationInitStatus], (status: ApplicationInitStatus) => { - let completerResolver = false; + status.runInitializers(); + setTimeout(() => { completerResolver = true; resolve(null); diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index a3f49184a7..e3f4a3fdff 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -225,11 +225,9 @@ export function main() { [{provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}])) .then(() => expect(false).toBe(true), (e) => { expect(e).toBe('Test'); - // Note: if the modules throws an error during construction, - // we don't have an injector and therefore no way of - // getting the exception handler. So - // the error is only rethrown but not logged via the exception handler. - expect(mockConsole.res).toEqual([]); + // Error rethrown will be seen by the exception handler since it's after + // construction. + expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test'); }); })); @@ -322,11 +320,9 @@ export function main() { const moduleFactory = compilerFactory.createCompiler().compileModuleSync(createModule( [{provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}])); expect(() => defaultPlatform.bootstrapModuleFactory(moduleFactory)).toThrow('Test'); - // Note: if the modules throws an error during construction, - // we don't have an injector and therefore no way of - // getting the exception handler. So - // the error is only rethrown but not logged via the exception handler. - expect(mockConsole.res).toEqual([]); + // Error rethrown will be seen by the exception handler since it's after + // construction. + expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test'); })); it('should rethrow promise errors even if the exceptionHandler is not rethrowing', diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 306f0a3d46..962dd11126 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CompilerOptions, Component, Directive, InjectionToken, Injector, ModuleWithComponentFactories, NgModule, NgModuleFactory, NgModuleRef, NgZone, Optional, Pipe, PlatformRef, Provider, ReflectiveInjector, SchemaMetadata, SkipSelf, Type, ɵDepFlags as DepFlags, ɵERROR_COMPONENT_TYPE, ɵNodeFlags as NodeFlags, ɵclearProviderOverrides as clearProviderOverrides, ɵoverrideProvider as overrideProvider, ɵstringify as stringify} from '@angular/core'; +import {ApplicationInitStatus, CompilerOptions, Component, Directive, InjectionToken, Injector, ModuleWithComponentFactories, NgModule, NgModuleFactory, NgModuleRef, NgZone, Optional, Pipe, PlatformRef, Provider, ReflectiveInjector, SchemaMetadata, SkipSelf, Type, ɵDepFlags as DepFlags, ɵERROR_COMPONENT_TYPE, ɵNodeFlags as NodeFlags, ɵclearProviderOverrides as clearProviderOverrides, ɵoverrideProvider as overrideProvider, ɵstringify as stringify} from '@angular/core'; import {AsyncTestCompleter} from './async_test_completer'; import {ComponentFixture} from './component_fixture'; @@ -311,6 +311,9 @@ export class TestBed implements Injector { const ngZoneInjector = ReflectiveInjector.resolveAndCreate( [{provide: NgZone, useValue: ngZone}], this.platform.injector); this._moduleRef = this._moduleFactory.create(ngZoneInjector); + // ApplicationInitStatus.runInitializers() is marked @internal to core. So casting to any + // before accessing it. + (this._moduleRef.injector.get(ApplicationInitStatus) as any).runInitializers(); this._instantiated = true; } diff --git a/packages/platform-browser/src/browser/server-transition.ts b/packages/platform-browser/src/browser/server-transition.ts index 83d3c12518..2bfb9de470 100644 --- a/packages/platform-browser/src/browser/server-transition.ts +++ b/packages/platform-browser/src/browser/server-transition.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_INITIALIZER, Inject, InjectionToken, Provider} from '@angular/core'; +import {APP_INITIALIZER, ApplicationInitStatus, Inject, InjectionToken, Injector, Provider} from '@angular/core'; import {getDOM} from '../dom/dom_adapter'; import {DOCUMENT} from '../dom/dom_tokens'; @@ -17,22 +17,25 @@ import {DOCUMENT} from '../dom/dom_tokens'; */ export const TRANSITION_ID = new InjectionToken('TRANSITION_ID'); -export function bootstrapListenerFactory(transitionId: string, document: any) { - const factory = () => { - const dom = getDOM(); - const styles: any[] = - Array.prototype.slice.apply(dom.querySelectorAll(document, `style[ng-transition]`)); - styles.filter(el => dom.getAttribute(el, 'ng-transition') === transitionId) - .forEach(el => dom.remove(el)); +export function appInitializerFactory(transitionId: string, document: any, injector: Injector) { + return () => { + // Wait for all application initializers to be completed before removing the styles set by + // the server. + injector.get(ApplicationInitStatus).donePromise.then(() => { + const dom = getDOM(); + const styles: any[] = + Array.prototype.slice.apply(dom.querySelectorAll(document, `style[ng-transition]`)); + styles.filter(el => dom.getAttribute(el, 'ng-transition') === transitionId) + .forEach(el => dom.remove(el)); + }); }; - return factory; } export const SERVER_TRANSITION_PROVIDERS: Provider[] = [ { provide: APP_INITIALIZER, - useFactory: bootstrapListenerFactory, - deps: [TRANSITION_ID, DOCUMENT], + useFactory: appInitializerFactory, + deps: [TRANSITION_ID, DOCUMENT, Injector], multi: true }, ];