From 3152d065dbe458d6fcb942abcb45433afe5f7971 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Thu, 25 May 2023 10:39:00 +0000 Subject: [PATCH] fix(core): update `ApplicationRef.isStable` to account for rendering pending tasks This commit updates the `ApplicationRef.isStable` API to account for pending rendering task. This is needed as once a pending rendering task is done, new macrotask and microtask could be created which previously caused these not to be intercepted and thus ignored when doing SSR. --- packages/common/http/src/transfer_cache.ts | 9 ++-- packages/core/src/application_ref.ts | 9 +++- packages/core/src/hydration/api.ts | 11 ++--- .../core/src/initial_render_pending_tasks.ts | 42 +++++-------------- .../initial_render_pending_tasks_spec.ts | 27 ++++-------- .../bundle.golden_symbols.json | 6 +++ .../animations/bundle.golden_symbols.json | 6 +++ .../cyclic_import/bundle.golden_symbols.json | 6 +++ .../forms_reactive/bundle.golden_symbols.json | 3 ++ .../bundle.golden_symbols.json | 3 ++ .../hello_world/bundle.golden_symbols.json | 6 +++ .../hydration/bundle.golden_symbols.json | 3 ++ .../bundle.golden_symbols.json | 6 +++ .../bundling/todo/bundle.golden_symbols.json | 6 +++ packages/platform-server/src/utils.ts | 6 +-- .../platform-server/test/hydration_spec.ts | 6 +-- 16 files changed, 79 insertions(+), 76 deletions(-) diff --git a/packages/common/http/src/transfer_cache.ts b/packages/common/http/src/transfer_cache.ts index 8d1e9130b7fd4b..a488ca5f7675e4 100644 --- a/packages/common/http/src/transfer_cache.ts +++ b/packages/common/http/src/transfer_cache.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_BOOTSTRAP_LISTENER, ApplicationRef, inject, InjectionToken, makeStateKey, Provider, StateKey, TransferState, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES, ɵInitialRenderPendingTasks as InitialRenderPendingTasks} from '@angular/core'; +import {APP_BOOTSTRAP_LISTENER, ApplicationRef, inject, InjectionToken, makeStateKey, Provider, StateKey, TransferState, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES} from '@angular/core'; import {Observable, of} from 'rxjs'; import {first, tap} from 'rxjs/operators'; @@ -165,16 +165,13 @@ export function withHttpTransferCache(): Provider[] { useFactory: () => { const appRef = inject(ApplicationRef); const cacheState = inject(CACHE_STATE); - const pendingTasks = inject(InitialRenderPendingTasks); return () => { - const isStablePromise = appRef.isStable.pipe(first((isStable) => isStable)).toPromise(); - isStablePromise.then(() => pendingTasks.whenAllTasksComplete).then(() => { + appRef.isStable.pipe(first((isStable) => isStable)).toPromise().then(() => { cacheState.isCacheActive = false; }); }; - }, - deps: [ApplicationRef, CACHE_STATE, InitialRenderPendingTasks] + } } ]; } diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index 7c916deca9de95..1d8489908ce1bb 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -9,6 +9,7 @@ import './util/ng_jit_mode'; import {Subscription} from 'rxjs'; +import {map} from 'rxjs/operators'; import {ApplicationInitStatus} from './application_init'; import {PLATFORM_INITIALIZER} from './application_tokens'; @@ -25,6 +26,7 @@ import {ErrorHandler} from './error_handler'; import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from './errors'; import {DEFAULT_LOCALE_ID} from './i18n/localization'; import {LOCALE_ID} from './i18n/tokens'; +import {InitialRenderPendingTasks} from './initial_render_pending_tasks'; import {Type} from './interface/type'; import {COMPILER_OPTIONS, CompilerOptions} from './linker/compiler'; import {ComponentFactory, ComponentRef} from './linker/component_factory'; @@ -38,7 +40,7 @@ import {isStandalone} from './render3/definition'; import {assertStandaloneComponentType} from './render3/errors'; import {setLocaleId} from './render3/i18n/i18n_locale_id'; import {setJitOptions} from './render3/jit/jit_options'; -import {createEnvironmentInjector, createNgModuleRefWithProviders, EnvironmentNgModuleRefAdapter, NgModuleFactory as R3NgModuleFactory} from './render3/ng_module_ref'; +import {createNgModuleRefWithProviders, EnvironmentNgModuleRefAdapter, NgModuleFactory as R3NgModuleFactory} from './render3/ng_module_ref'; import {publishDefaultGlobalUtils as _publishDefaultGlobalUtils} from './render3/util/global_utils'; import {setThrowInvalidWriteToSignalError} from './signals'; import {TESTABILITY} from './testability/testability'; @@ -819,6 +821,7 @@ export class ApplicationRef { /** @internal */ _views: InternalViewRef[] = []; private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER); + private readonly pendingTasks = inject(InitialRenderPendingTasks); /** * Indicates whether this instance was destroyed. @@ -841,7 +844,9 @@ export class ApplicationRef { /** * Returns an Observable that indicates when the application is stable or unstable. */ - public readonly isStable = inject(ZONE_IS_STABLE_OBSERVABLE); + public readonly isStable = + inject(ZONE_IS_STABLE_OBSERVABLE) + .pipe(map(isStable => isStable && !this.pendingTasks.hasPendingTasks)); private readonly _injector = inject(EnvironmentInjector); /** diff --git a/packages/core/src/hydration/api.ts b/packages/core/src/hydration/api.ts index 20f463fc775069..1ea53959fc69df 100644 --- a/packages/core/src/hydration/api.ts +++ b/packages/core/src/hydration/api.ts @@ -14,7 +14,6 @@ import {Console} from '../console'; import {ENVIRONMENT_INITIALIZER, EnvironmentProviders, Injector, makeEnvironmentProviders} from '../di'; import {inject} from '../di/injector_compatibility'; import {formatRuntimeError, RuntimeErrorCode} from '../errors'; -import {InitialRenderPendingTasks} from '../initial_render_pending_tasks'; import {enableLocateOrCreateContainerRefImpl} from '../linker/view_container_ref'; import {enableLocateOrCreateElementNodeImpl} from '../render3/instructions/element'; import {enableLocateOrCreateElementContainerNodeImpl} from '../render3/instructions/element_container'; @@ -94,9 +93,7 @@ function printHydrationStats(injector: Injector) { /** * Returns a Promise that is resolved when an application becomes stable. */ -function whenStable( - appRef: ApplicationRef, pendingTasks: InitialRenderPendingTasks, - injector: Injector): Promise { +function whenStable(appRef: ApplicationRef, injector: Injector): Promise { const isStablePromise = appRef.isStable.pipe(first((isStable: boolean) => isStable)).toPromise(); if (typeof ngDevMode !== 'undefined' && ngDevMode) { const timeoutTime = APPLICATION_IS_STABLE_TIMEOUT; @@ -113,8 +110,7 @@ function whenStable( isStablePromise.finally(() => clearTimeout(timeoutId)); } - const pendingTasksPromise = pendingTasks.whenAllTasksComplete; - return Promise.allSettled([isStablePromise, pendingTasksPromise]); + return isStablePromise.then(() => {}); } /** @@ -186,10 +182,9 @@ export function withDomHydration(): EnvironmentProviders { useFactory: () => { if (isBrowser() && inject(IS_HYDRATION_DOM_REUSE_ENABLED)) { const appRef = inject(ApplicationRef); - const pendingTasks = inject(InitialRenderPendingTasks); const injector = inject(Injector); return () => { - whenStable(appRef, pendingTasks, injector).then(() => { + whenStable(appRef, injector).then(() => { // Wait until an app becomes stable and cleanup all views that // were not claimed during the application bootstrap process. // The timing is similar to when we start the serialization process diff --git a/packages/core/src/initial_render_pending_tasks.ts b/packages/core/src/initial_render_pending_tasks.ts index 6a79aa19ac2a21..170f6eec32681d 100644 --- a/packages/core/src/initial_render_pending_tasks.ts +++ b/packages/core/src/initial_render_pending_tasks.ts @@ -7,9 +7,7 @@ */ import {Injectable} from './di'; -import {inject} from './di/injector_compatibility'; import {OnDestroy} from './interface/lifecycle_hooks'; -import {NgZone} from './zone/ng_zone'; /** * *Internal* service that keeps track of pending tasks happening in the system @@ -23,30 +21,11 @@ import {NgZone} from './zone/ng_zone'; @Injectable({providedIn: 'root'}) export class InitialRenderPendingTasks implements OnDestroy { private taskId = 0; - private collection = new Set(); - private ngZone = inject(NgZone); + private pendingTasks = new Set(); + private completed = false; - private resolve!: VoidFunction; - private promise!: Promise; - - get whenAllTasksComplete(): Promise { - if (this.collection.size === 0) { - this.complete(); - } - - return this.promise; - } - - completed = false; - - constructor() { - // Run outside of the Angular zone to avoid triggering - // extra change detection cycles. - this.ngZone.runOutsideAngular(() => { - this.promise = new Promise((resolve) => { - this.resolve = resolve; - }); - }); + get hasPendingTasks(): boolean { + return this.pendingTasks.size > 0; } add(): number { @@ -56,26 +35,25 @@ export class InitialRenderPendingTasks implements OnDestroy { return -1; } const taskId = this.taskId++; - this.collection.add(taskId); + this.pendingTasks.add(taskId); return taskId; } - remove(taskId: number) { + remove(taskId: number): void { if (this.completed) return; - this.collection.delete(taskId); - if (this.collection.size === 0) { + this.pendingTasks.delete(taskId); + if (this.pendingTasks.size === 0) { this.complete(); } } - ngOnDestroy() { + ngOnDestroy(): void { this.complete(); - this.collection.clear(); } private complete(): void { this.completed = true; - this.resolve(); + this.pendingTasks.clear(); } } diff --git a/packages/core/test/acceptance/initial_render_pending_tasks_spec.ts b/packages/core/test/acceptance/initial_render_pending_tasks_spec.ts index 5089b6a04a9c11..56ce329a27eaf0 100644 --- a/packages/core/test/acceptance/initial_render_pending_tasks_spec.ts +++ b/packages/core/test/acceptance/initial_render_pending_tasks_spec.ts @@ -11,54 +11,43 @@ import {TestBed} from '@angular/core/testing'; import {InitialRenderPendingTasks} from '../../src/initial_render_pending_tasks'; describe('InitialRenderPendingTasks', () => { - it('should resolve a promise even if there are no tasks', async () => { - const pendingTasks = TestBed.inject(InitialRenderPendingTasks); - expect(pendingTasks.completed).toBe(false); - await pendingTasks.whenAllTasksComplete; - expect(pendingTasks.completed).toBe(true); - }); - it('should wait until all tasks are completed', async () => { const pendingTasks = TestBed.inject(InitialRenderPendingTasks); - expect(pendingTasks.completed).toBe(false); + expect(pendingTasks.hasPendingTasks).toBeFalse(); const taskA = pendingTasks.add(); const taskB = pendingTasks.add(); const taskC = pendingTasks.add(); - expect(pendingTasks.completed).toBe(false); + expect(pendingTasks.hasPendingTasks).toBeTrue(); pendingTasks.remove(taskA); pendingTasks.remove(taskB); pendingTasks.remove(taskC); - await pendingTasks.whenAllTasksComplete; - expect(pendingTasks.completed).toBe(true); + expect(pendingTasks.hasPendingTasks).toBeFalse(); }); it('should allow calls to remove the same task multiple times', async () => { const pendingTasks = TestBed.inject(InitialRenderPendingTasks); - expect(pendingTasks.completed).toBe(false); + expect(pendingTasks.hasPendingTasks).toBeFalse(); const taskA = pendingTasks.add(); - expect(pendingTasks.completed).toBe(false); + expect(pendingTasks.hasPendingTasks).toBeTrue(); pendingTasks.remove(taskA); pendingTasks.remove(taskA); pendingTasks.remove(taskA); - - await pendingTasks.whenAllTasksComplete; - expect(pendingTasks.completed).toBe(true); + expect(pendingTasks.hasPendingTasks).toBeFalse(); }); it('should be tolerant to removal of non-existent ids', async () => { const pendingTasks = TestBed.inject(InitialRenderPendingTasks); - expect(pendingTasks.completed).toBe(false); + expect(pendingTasks.hasPendingTasks).toBeFalse(); pendingTasks.remove(Math.random()); pendingTasks.remove(Math.random()); pendingTasks.remove(Math.random()); - await pendingTasks.whenAllTasksComplete; - expect(pendingTasks.completed).toBe(true); + expect(pendingTasks.hasPendingTasks).toBeFalse(); }); }); diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index f724d3b02e83c7..bcafd01a62538d 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -242,6 +242,9 @@ { "name": "INTERNAL_BROWSER_PLATFORM_PROVIDERS" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -1172,6 +1175,9 @@ { "name": "makeTimingAst" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 6d4cc476ab6d39..8c1307fd4f1474 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -263,6 +263,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -1238,6 +1241,9 @@ { "name": "makeTimingAst" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 24ce510edc27ff..f5dbfcdaf8fa14 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -167,6 +167,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -977,6 +980,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 432b39db4af770..15d2fac6411ff7 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -245,6 +245,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index bce672adb47ddc..ee046b9a0b70b8 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -230,6 +230,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 530a3ecde2e638..093bfa6d69e12a 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -110,6 +110,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -764,6 +767,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 48e29e255bf4c4..d0e877443631b5 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -1052,6 +1052,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 86bb9f57ad7138..fad88d75c2db85 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -152,6 +152,9 @@ { "name": "INTERNAL_BROWSER_PLATFORM_PROVIDERS" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -857,6 +860,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index ecec976de4c795..2d3c5feebeb78f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -170,6 +170,9 @@ { "name": "INTERNAL_APPLICATION_ERROR_HANDLER" }, + { + "name": "InitialRenderPendingTasks" + }, { "name": "InjectFlags" }, @@ -1172,6 +1175,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, diff --git a/packages/platform-server/src/utils.ts b/packages/platform-server/src/utils.ts index 12177666e57dcc..807b983317c77e 100644 --- a/packages/platform-server/src/utils.ts +++ b/packages/platform-server/src/utils.ts @@ -54,13 +54,9 @@ function appendServerContextInfo(applicationRef: ApplicationRef) { async function _render(platformRef: PlatformRef, applicationRef: ApplicationRef): Promise { const environmentInjector = applicationRef.injector; - const isStablePromise = - applicationRef.isStable.pipe((first((isStable: boolean) => isStable))).toPromise(); - const pendingTasks = environmentInjector.get(InitialRenderPendingTasks); - const pendingTasksPromise = pendingTasks.whenAllTasksComplete; // Block until application is stable. - await Promise.allSettled([isStablePromise, pendingTasksPromise]); + await applicationRef.isStable.pipe((first((isStable: boolean) => isStable))).toPromise(); const platformState = platformRef.injector.get(PlatformState); if (applicationRef.injector.get(IS_HYDRATION_DOM_REUSE_ENABLED, false)) { diff --git a/packages/platform-server/test/hydration_spec.ts b/packages/platform-server/test/hydration_spec.ts index 75db97fdb0f2b7..26a3b8a0703129 100644 --- a/packages/platform-server/test/hydration_spec.ts +++ b/packages/platform-server/test/hydration_spec.ts @@ -99,10 +99,8 @@ function stripExcessiveSpaces(html: string): string { } /** Returns a Promise that resolves when the ApplicationRef becomes stable. */ -function whenStable(appRef: ApplicationRef): Promise { - const isStablePromise = appRef.isStable.pipe(first((isStable: boolean) => isStable)).toPromise(); - const pendingTasksPromise = appRef.injector.get(InitialRenderPendingTasks).whenAllTasksComplete; - return Promise.allSettled([isStablePromise, pendingTasksPromise]); +function whenStable(appRef: ApplicationRef): Promise { + return appRef.isStable.pipe(first((isStable: boolean) => isStable)).toPromise().then(() => {}); } function verifyClientAndSSRContentsMatch(ssrContents: string, clientAppRootElement: HTMLElement) {