Skip to content

Commit 28c68f7

Browse files
alan-agius4dylhunn
alan-agius4
authored and
dylhunn
committed
fix(core): update ApplicationRef.isStable to account for rendering 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
1 parent 381cb98 commit 28c68f7

File tree

18 files changed

+214
-100
lines changed

18 files changed

+214
-100
lines changed

‎goldens/size-tracking/integration-payloads.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"standalone-bootstrap": {
5353
"uncompressed": {
5454
"runtime": 918,
55-
"main": 87081,
55+
"main": 91485,
5656
"polyfills": 33802
5757
}
5858
}

‎packages/common/http/src/transfer_cache.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {APP_BOOTSTRAP_LISTENER, ApplicationRef, inject, InjectionToken, makeStateKey, Provider, StateKey, TransferState, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES, ɵInitialRenderPendingTasks as InitialRenderPendingTasks} from '@angular/core';
9+
import {APP_BOOTSTRAP_LISTENER, ApplicationRef, inject, InjectionToken, makeStateKey, Provider, StateKey, TransferState, ɵENABLED_SSR_FEATURES as ENABLED_SSR_FEATURES} from '@angular/core';
1010
import {Observable, of} from 'rxjs';
1111
import {first, tap} from 'rxjs/operators';
1212

@@ -165,16 +165,13 @@ export function withHttpTransferCache(): Provider[] {
165165
useFactory: () => {
166166
const appRef = inject(ApplicationRef);
167167
const cacheState = inject(CACHE_STATE);
168-
const pendingTasks = inject(InitialRenderPendingTasks);
169168

170169
return () => {
171-
const isStablePromise = appRef.isStable.pipe(first((isStable) => isStable)).toPromise();
172-
isStablePromise.then(() => pendingTasks.whenAllTasksComplete).then(() => {
170+
appRef.isStable.pipe(first((isStable) => isStable)).toPromise().then(() => {
173171
cacheState.isCacheActive = false;
174172
});
175173
};
176-
},
177-
deps: [ApplicationRef, CACHE_STATE, InitialRenderPendingTasks]
174+
}
178175
}
179176
];
180177
}

‎packages/core/src/application_ref.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
import './util/ng_jit_mode';
1010

11-
import {Subscription} from 'rxjs';
11+
import {Observable, of, Subscription} from 'rxjs';
12+
import {distinctUntilChanged, mergeMap, share} from 'rxjs/operators';
1213

1314
import {ApplicationInitStatus} from './application_init';
1415
import {PLATFORM_INITIALIZER} from './application_tokens';
@@ -25,6 +26,7 @@ import {ErrorHandler} from './error_handler';
2526
import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from './errors';
2627
import {DEFAULT_LOCALE_ID} from './i18n/localization';
2728
import {LOCALE_ID} from './i18n/tokens';
29+
import {InitialRenderPendingTasks} from './initial_render_pending_tasks';
2830
import {Type} from './interface/type';
2931
import {COMPILER_OPTIONS, CompilerOptions} from './linker/compiler';
3032
import {ComponentFactory, ComponentRef} from './linker/component_factory';
@@ -38,7 +40,7 @@ import {isStandalone} from './render3/definition';
3840
import {assertStandaloneComponentType} from './render3/errors';
3941
import {setLocaleId} from './render3/i18n/i18n_locale_id';
4042
import {setJitOptions} from './render3/jit/jit_options';
41-
import {createEnvironmentInjector, createNgModuleRefWithProviders, EnvironmentNgModuleRefAdapter, NgModuleFactory as R3NgModuleFactory} from './render3/ng_module_ref';
43+
import {createNgModuleRefWithProviders, EnvironmentNgModuleRefAdapter, NgModuleFactory as R3NgModuleFactory} from './render3/ng_module_ref';
4244
import {publishDefaultGlobalUtils as _publishDefaultGlobalUtils} from './render3/util/global_utils';
4345
import {setThrowInvalidWriteToSignalError} from './signals';
4446
import {TESTABILITY} from './testability/testability';
@@ -819,6 +821,7 @@ export class ApplicationRef {
819821
/** @internal */
820822
_views: InternalViewRef[] = [];
821823
private readonly internalErrorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
824+
private readonly zoneIsStable = inject(ZONE_IS_STABLE_OBSERVABLE);
822825

823826
/**
824827
* Indicates whether this instance was destroyed.
@@ -841,7 +844,13 @@ export class ApplicationRef {
841844
/**
842845
* Returns an Observable that indicates when the application is stable or unstable.
843846
*/
844-
public readonly isStable = inject(ZONE_IS_STABLE_OBSERVABLE);
847+
public readonly isStable: Observable<boolean> =
848+
inject(InitialRenderPendingTasks)
849+
.hasPendingTasks.pipe(
850+
mergeMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
851+
distinctUntilChanged(),
852+
share(),
853+
);
845854

846855
private readonly _injector = inject(EnvironmentInjector);
847856
/**

‎packages/core/src/hydration/api.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {Console} from '../console';
1414
import {ENVIRONMENT_INITIALIZER, EnvironmentProviders, Injector, makeEnvironmentProviders} from '../di';
1515
import {inject} from '../di/injector_compatibility';
1616
import {formatRuntimeError, RuntimeErrorCode} from '../errors';
17-
import {InitialRenderPendingTasks} from '../initial_render_pending_tasks';
1817
import {enableLocateOrCreateContainerRefImpl} from '../linker/view_container_ref';
1918
import {enableLocateOrCreateElementNodeImpl} from '../render3/instructions/element';
2019
import {enableLocateOrCreateElementContainerNodeImpl} from '../render3/instructions/element_container';
@@ -94,9 +93,7 @@ function printHydrationStats(injector: Injector) {
9493
/**
9594
* Returns a Promise that is resolved when an application becomes stable.
9695
*/
97-
function whenStable(
98-
appRef: ApplicationRef, pendingTasks: InitialRenderPendingTasks,
99-
injector: Injector): Promise<unknown> {
96+
function whenStable(appRef: ApplicationRef, injector: Injector): Promise<void> {
10097
const isStablePromise = appRef.isStable.pipe(first((isStable: boolean) => isStable)).toPromise();
10198
if (typeof ngDevMode !== 'undefined' && ngDevMode) {
10299
const timeoutTime = APPLICATION_IS_STABLE_TIMEOUT;
@@ -113,8 +110,7 @@ function whenStable(
113110
isStablePromise.finally(() => clearTimeout(timeoutId));
114111
}
115112

116-
const pendingTasksPromise = pendingTasks.whenAllTasksComplete;
117-
return Promise.allSettled([isStablePromise, pendingTasksPromise]);
113+
return isStablePromise.then(() => {});
118114
}
119115

120116
/**
@@ -186,10 +182,9 @@ export function withDomHydration(): EnvironmentProviders {
186182
useFactory: () => {
187183
if (isBrowser() && inject(IS_HYDRATION_DOM_REUSE_ENABLED)) {
188184
const appRef = inject(ApplicationRef);
189-
const pendingTasks = inject(InitialRenderPendingTasks);
190185
const injector = inject(Injector);
191186
return () => {
192-
whenStable(appRef, pendingTasks, injector).then(() => {
187+
whenStable(appRef, injector).then(() => {
193188
// Wait until an app becomes stable and cleanup all views that
194189
// were not claimed during the application bootstrap process.
195190
// The timing is similar to when we start the serialization process

‎packages/core/src/initial_render_pending_tasks.ts‎

Lines changed: 13 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import {BehaviorSubject} from 'rxjs';
10+
911
import {Injectable} from './di';
10-
import {inject} from './di/injector_compatibility';
1112
import {OnDestroy} from './interface/lifecycle_hooks';
12-
import {NgZone} from './zone/ng_zone';
1313

1414
/**
1515
* *Internal* service that keeps track of pending tasks happening in the system
@@ -23,59 +23,25 @@ import {NgZone} from './zone/ng_zone';
2323
@Injectable({providedIn: 'root'})
2424
export class InitialRenderPendingTasks implements OnDestroy {
2525
private taskId = 0;
26-
private collection = new Set<number>();
27-
private ngZone = inject(NgZone);
28-
29-
private resolve!: VoidFunction;
30-
private promise!: Promise<void>;
31-
32-
get whenAllTasksComplete(): Promise<void> {
33-
if (this.collection.size === 0) {
34-
this.complete();
35-
}
36-
37-
return this.promise;
38-
}
39-
40-
completed = false;
41-
42-
constructor() {
43-
// Run outside of the Angular zone to avoid triggering
44-
// extra change detection cycles.
45-
this.ngZone.runOutsideAngular(() => {
46-
this.promise = new Promise<void>((resolve) => {
47-
this.resolve = resolve;
48-
});
49-
});
50-
}
26+
private pendingTasks = new Set<number>();
27+
hasPendingTasks = new BehaviorSubject<boolean>(false);
5128

5229
add(): number {
53-
if (this.completed) {
54-
// Indicates that the task was added after
55-
// the task queue completion, so it's a noop.
56-
return -1;
57-
}
30+
this.hasPendingTasks.next(true);
5831
const taskId = this.taskId++;
59-
this.collection.add(taskId);
32+
this.pendingTasks.add(taskId);
6033
return taskId;
6134
}
6235

63-
remove(taskId: number) {
64-
if (this.completed) return;
65-
66-
this.collection.delete(taskId);
67-
if (this.collection.size === 0) {
68-
this.complete();
36+
remove(taskId: number): void {
37+
this.pendingTasks.delete(taskId);
38+
if (this.pendingTasks.size === 0) {
39+
this.hasPendingTasks.next(false);
6940
}
7041
}
7142

72-
ngOnDestroy() {
73-
this.complete();
74-
this.collection.clear();
75-
}
76-
77-
private complete(): void {
78-
this.completed = true;
79-
this.resolve();
43+
ngOnDestroy(): void {
44+
this.pendingTasks.clear();
45+
this.hasPendingTasks.next(false);
8046
}
8147
}

‎packages/core/test/acceptance/initial_render_pending_tasks_spec.ts‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,58 +7,55 @@
77
*/
88

99
import {TestBed} from '@angular/core/testing';
10+
import {EMPTY, of} from 'rxjs';
11+
import {map, withLatestFrom} from 'rxjs/operators';
1012

1113
import {InitialRenderPendingTasks} from '../../src/initial_render_pending_tasks';
1214

1315
describe('InitialRenderPendingTasks', () => {
14-
it('should resolve a promise even if there are no tasks', async () => {
15-
const pendingTasks = TestBed.inject(InitialRenderPendingTasks);
16-
expect(pendingTasks.completed).toBe(false);
17-
await pendingTasks.whenAllTasksComplete;
18-
expect(pendingTasks.completed).toBe(true);
19-
});
20-
2116
it('should wait until all tasks are completed', async () => {
2217
const pendingTasks = TestBed.inject(InitialRenderPendingTasks);
23-
expect(pendingTasks.completed).toBe(false);
24-
2518
const taskA = pendingTasks.add();
2619
const taskB = pendingTasks.add();
2720
const taskC = pendingTasks.add();
28-
expect(pendingTasks.completed).toBe(false);
2921

3022
pendingTasks.remove(taskA);
3123
pendingTasks.remove(taskB);
3224
pendingTasks.remove(taskC);
33-
await pendingTasks.whenAllTasksComplete;
34-
expect(pendingTasks.completed).toBe(true);
25+
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
3526
});
3627

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

4132
const taskA = pendingTasks.add();
42-
43-
expect(pendingTasks.completed).toBe(false);
33+
expect(await hasPendingTasks(pendingTasks)).toBeTrue();
4434

4535
pendingTasks.remove(taskA);
4636
pendingTasks.remove(taskA);
4737
pendingTasks.remove(taskA);
4838

49-
await pendingTasks.whenAllTasksComplete;
50-
expect(pendingTasks.completed).toBe(true);
39+
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
5140
});
5241

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

5746
pendingTasks.remove(Math.random());
5847
pendingTasks.remove(Math.random());
5948
pendingTasks.remove(Math.random());
6049

61-
await pendingTasks.whenAllTasksComplete;
62-
expect(pendingTasks.completed).toBe(true);
50+
expect(await hasPendingTasks(pendingTasks)).toBeFalse();
6351
});
6452
});
53+
54+
function hasPendingTasks(pendingTasks: InitialRenderPendingTasks): Promise<boolean> {
55+
return of(EMPTY)
56+
.pipe(
57+
withLatestFrom(pendingTasks.hasPendingTasks),
58+
map(([_, hasPendingTasks]) => hasPendingTasks),
59+
)
60+
.toPromise();
61+
}

‎packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@
8080
{
8181
"name": "BaseAnimationRenderer"
8282
},
83+
{
84+
"name": "BehaviorSubject"
85+
},
8386
{
8487
"name": "BrowserAnimationBuilder"
8588
},
@@ -155,6 +158,9 @@
155158
{
156159
"name": "DefaultDomRenderer2"
157160
},
161+
{
162+
"name": "DistinctUntilChangedSubscriber"
163+
},
158164
{
159165
"name": "DomAdapter"
160166
},
@@ -242,6 +248,9 @@
242248
{
243249
"name": "INTERNAL_BROWSER_PLATFORM_PROVIDERS"
244250
},
251+
{
252+
"name": "InitialRenderPendingTasks"
253+
},
245254
{
246255
"name": "InjectFlags"
247256
},
@@ -854,6 +863,9 @@
854863
{
855864
"name": "forwardRef"
856865
},
866+
{
867+
"name": "fromArray"
868+
},
857869
{
858870
"name": "generateInitialInputs"
859871
},
@@ -1121,6 +1133,9 @@
11211133
{
11221134
"name": "isPromise2"
11231135
},
1136+
{
1137+
"name": "isScheduler"
1138+
},
11241139
{
11251140
"name": "isStableFactory"
11261141
},
@@ -1379,6 +1394,9 @@
13791394
{
13801395
"name": "setupStaticAttributes"
13811396
},
1397+
{
1398+
"name": "share"
1399+
},
13821400
{
13831401
"name": "shareSubjectFactory"
13841402
},

0 commit comments

Comments
 (0)