Skip to content

Commit

Permalink
fix(core): update ApplicationRef.isStable to account for rendering …
Browse files Browse the repository at this point in the history 
…pending tasks (#50425)

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.

PR Close #50425
  • Loading branch information
@alan-agius4 @dylhunn
alan-agius4 authored and dylhunn committed May 30, 2023
1 parent 381cb98 commit 28c68f7
Show file tree
Hide file tree
Showing 18 changed files with 214 additions and 100 deletions.
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
View file
Expand Up @@ -52,7 +52,7 @@
"standalone-bootstrap": {
"uncompressed": {
"runtime": 918,
"main": 87081,
"main": 91485,
"polyfills": 33802
}
}
Expand Down
9 changes: 3 additions & 6 deletions packages/common/http/src/transfer_cache.ts
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -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]
}
}
];
}
15 changes: 12 additions & 3 deletions packages/core/src/application_ref.ts
View file
Expand Up @@ -8,7 +8,8 @@

import './util/ng_jit_mode';

import {Subscription} from 'rxjs';
import {Observable, of, Subscription} from 'rxjs';
import {distinctUntilChanged, mergeMap, share} from 'rxjs/operators';

import {ApplicationInitStatus} from './application_init';
import {PLATFORM_INITIALIZER} from './application_tokens';
Expand All @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -819,6 +821,7 @@ export class ApplicationRef {
/** @internal */
_views: InternalViewRef[] = [];
private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
private readonly zoneIsStable = inject(ZONE_IS_STABLE_OBSERVABLE);

/**
* Indicates whether this instance was destroyed.
Expand All @@ -841,7 +844,13 @@ 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: Observable<boolean> =
inject(InitialRenderPendingTasks)
.hasPendingTasks.pipe(
mergeMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
distinctUntilChanged(),
share(),
);

private readonly _injector = inject(EnvironmentInjector);
/**
Expand Down
11 changes: 3 additions & 8 deletions packages/core/src/hydration/api.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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<unknown> {
function whenStable(appRef: ApplicationRef, injector: Injector): Promise<void> {
const isStablePromise = appRef.isStable.pipe(first((isStable: boolean) => isStable)).toPromise();
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
const timeoutTime = APPLICATION_IS_STABLE_TIMEOUT;
Expand All @@ -113,8 +110,7 @@ function whenStable(
isStablePromise.finally(() => clearTimeout(timeoutId));
}

const pendingTasksPromise = pendingTasks.whenAllTasksComplete;
return Promise.allSettled([isStablePromise, pendingTasksPromise]);
return isStablePromise.then(() => {});
}

/**
Expand Down Expand Up @@ -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
Expand Down
60 changes: 13 additions & 47 deletions packages/core/src/initial_render_pending_tasks.ts
View file
Expand Up @@ -6,10 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {BehaviorSubject} from 'rxjs';

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
Expand All @@ -23,59 +23,25 @@ import {NgZone} from './zone/ng_zone';
@Injectable({providedIn: 'root'})
export class InitialRenderPendingTasks implements OnDestroy {
private taskId = 0;
private collection = new Set<number>();
private ngZone = inject(NgZone);

private resolve!: VoidFunction;
private promise!: Promise<void>;

get whenAllTasksComplete(): Promise<void> {
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<void>((resolve) => {
this.resolve = resolve;
});
});
}
private pendingTasks = new Set<number>();
hasPendingTasks = new BehaviorSubject<boolean>(false);

add(): number {
if (this.completed) {
// Indicates that the task was added after
// the task queue completion, so it's a noop.
return -1;
}
this.hasPendingTasks.next(true);
const taskId = this.taskId++;
this.collection.add(taskId);
this.pendingTasks.add(taskId);
return taskId;
}

remove(taskId: number) {
if (this.completed) return;

this.collection.delete(taskId);
if (this.collection.size === 0) {
this.complete();
remove(taskId: number): void {
this.pendingTasks.delete(taskId);
if (this.pendingTasks.size === 0) {
this.hasPendingTasks.next(false);
}
}

ngOnDestroy() {
this.complete();
this.collection.clear();
}

private complete(): void {
this.completed = true;
this.resolve();
ngOnDestroy(): void {
this.pendingTasks.clear();
this.hasPendingTasks.next(false);
}
}
37 changes: 17 additions & 20 deletions packages/core/test/acceptance/initial_render_pending_tasks_spec.ts
View file
Expand Up @@ -7,58 +7,55 @@
*/

import {TestBed} from '@angular/core/testing';
import {EMPTY, of} from 'rxjs';
import {map, withLatestFrom} from 'rxjs/operators';

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);

const taskA = pendingTasks.add();
const taskB = pendingTasks.add();
const taskC = pendingTasks.add();
expect(pendingTasks.completed).toBe(false);

pendingTasks.remove(taskA);
pendingTasks.remove(taskB);
pendingTasks.remove(taskC);
await pendingTasks.whenAllTasksComplete;
expect(pendingTasks.completed).toBe(true);
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
});

it('should allow calls to remove the same task multiple times', async () => {
const pendingTasks = TestBed.inject(InitialRenderPendingTasks);
expect(pendingTasks.completed).toBe(false);
expect(await hasPendingTasks(pendingTasks)).toBeFalse();

const taskA = pendingTasks.add();

expect(pendingTasks.completed).toBe(false);
expect(await hasPendingTasks(pendingTasks)).toBeTrue();

pendingTasks.remove(taskA);
pendingTasks.remove(taskA);
pendingTasks.remove(taskA);

await pendingTasks.whenAllTasksComplete;
expect(pendingTasks.completed).toBe(true);
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
});

it('should be tolerant to removal of non-existent ids', async () => {
const pendingTasks = TestBed.inject(InitialRenderPendingTasks);
expect(pendingTasks.completed).toBe(false);
expect(await hasPendingTasks(pendingTasks)).toBeFalse();

pendingTasks.remove(Math.random());
pendingTasks.remove(Math.random());
pendingTasks.remove(Math.random());

await pendingTasks.whenAllTasksComplete;
expect(pendingTasks.completed).toBe(true);
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
});
});

function hasPendingTasks(pendingTasks: InitialRenderPendingTasks): Promise<boolean> {
return of(EMPTY)
.pipe(
withLatestFrom(pendingTasks.hasPendingTasks),
map(([_, hasPendingTasks]) => hasPendingTasks),
)
.toPromise();
}
18 changes: 18 additions & 0 deletions packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
View file
Expand Up @@ -80,6 +80,9 @@
{
"name": "BaseAnimationRenderer"
},
{
"name": "BehaviorSubject"
},
{
"name": "BrowserAnimationBuilder"
},
Expand Down Expand Up @@ -155,6 +158,9 @@
{
"name": "DefaultDomRenderer2"
},
{
"name": "DistinctUntilChangedSubscriber"
},
{
"name": "DomAdapter"
},
Expand Down Expand Up @@ -242,6 +248,9 @@
{
"name": "INTERNAL_BROWSER_PLATFORM_PROVIDERS"
},
{
"name": "InitialRenderPendingTasks"
},
{
"name": "InjectFlags"
},
Expand Down Expand Up @@ -854,6 +863,9 @@
{
"name": "forwardRef"
},
{
"name": "fromArray"
},
{
"name": "generateInitialInputs"
},
Expand Down Expand Up @@ -1121,6 +1133,9 @@
{
"name": "isPromise2"
},
{
"name": "isScheduler"
},
{
"name": "isStableFactory"
},
Expand Down Expand Up @@ -1379,6 +1394,9 @@
{
"name": "setupStaticAttributes"
},
{
"name": "share"
},
{
"name": "shareSubjectFactory"
},
Expand Down

0 comments on commit 28c68f7

Please sign in to comment.