From 400dbc5b89a2af0ae5fd7830f6ea47352c8556ef Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Tue, 29 Apr 2025 15:49:26 +0000 Subject: [PATCH] fix(core): properly handle app stabilization with defer blocks (#61056) Previously, the app was marked as stable prematurely. For more details, see https://github.com/angular/angular/issues/61038#issuecomment-2837917180 Closes: #61038 (cherry picked from commit de649c9cfd945b7d4f7df0533fa884a56a3d4158) PR Close #61056 --- .../e2e/src/defer-spec.ts | 3 +-- .../http-transferstate-lazy-on-init-spec.ts | 2 +- .../e2e/src/http-transferstate-lazy-spec.ts | 2 +- .../projects/standalone/prerender.ts | 26 +++---------------- .../projects/standalone/src/app/app.config.ts | 4 +-- .../projects/standalone/src/app/app.routes.ts | 5 ---- .../http-transfer-state-on-init.component.ts | 8 +++--- .../http-transfer-state.component.ts | 9 +++++-- packages/core/src/defer/triggering.ts | 20 +++++++------- .../bundling/defer/bundle.golden_symbols.json | 1 + 10 files changed, 32 insertions(+), 48 deletions(-) diff --git a/integration/platform-server-zoneless/e2e/src/defer-spec.ts b/integration/platform-server-zoneless/e2e/src/defer-spec.ts index ba6a91251ad..ac1862ff268 100644 --- a/integration/platform-server-zoneless/e2e/src/defer-spec.ts +++ b/integration/platform-server-zoneless/e2e/src/defer-spec.ts @@ -1,8 +1,7 @@ import {browser, by, element} from 'protractor'; import {bootstrapClientApp, navigateTo, verifyNoBrowserErrors} from './util'; -// TODO: this does not work with zoneless -xdescribe('Defer E2E Tests', () => { +describe('Defer E2E Tests', () => { beforeEach(async () => { // Don't wait for Angular since it is not bootstrapped automatically. await browser.waitForAngularEnabled(false); diff --git a/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-on-init-spec.ts b/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-on-init-spec.ts index 668a842ff71..a3d9b3494af 100644 --- a/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-on-init-spec.ts +++ b/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-on-init-spec.ts @@ -10,7 +10,7 @@ import {browser, by, element} from 'protractor'; import {bootstrapClientApp, navigateTo, verifyNoBrowserErrors} from './util'; // TODO: this does not work with zoneless -xdescribe('Http TransferState Lazy On Init', () => { +describe('Http TransferState Lazy On Init', () => { beforeEach(async () => { // Don't wait for Angular since it is not bootstrapped automatically. await browser.waitForAngularEnabled(false); diff --git a/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-spec.ts b/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-spec.ts index 2d9ffaae80a..4707f35065e 100644 --- a/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-spec.ts +++ b/integration/platform-server-zoneless/e2e/src/http-transferstate-lazy-spec.ts @@ -10,7 +10,7 @@ import {browser, by, element} from 'protractor'; import {bootstrapClientApp, navigateTo, verifyNoBrowserErrors} from './util'; // TODO: this does not work with zoneless -xdescribe('Http TransferState Lazy', () => { +describe('Http TransferState Lazy', () => { beforeEach(async () => { // Don't wait for Angular since it is not bootstrapped automatically. await browser.waitForAngularEnabled(false); diff --git a/integration/platform-server-zoneless/projects/standalone/prerender.ts b/integration/platform-server-zoneless/projects/standalone/prerender.ts index 90fa07eeff0..d7197ff6402 100644 --- a/integration/platform-server-zoneless/projects/standalone/prerender.ts +++ b/integration/platform-server-zoneless/projects/standalone/prerender.ts @@ -17,28 +17,10 @@ const browserDistFolder = resolve(serverDistFolder, '../browser'); const indexHtml = readFileSync(join(browserDistFolder, 'index.csr.html'), 'utf-8'); async function runTest() { - // Test and validate the errors are printed in the console. - const originalConsoleError = console.error; - const errors: string[] = []; - console.error = (error, data) => errors.push(error.toString() + ' ' + data.toString()); - - try { - await renderApplication(bootstrap, { - document: indexHtml, - url: '/error', - }); - } catch {} - - console.error = originalConsoleError; - - // Test case - if (!errors.some((e) => e.includes('Error in resolver'))) { - errors.forEach(console.error); - console.error( - '\nError: expected rendering errors ("Error in resolver") to be printed in the console.\n', - ); - process.exit(1); - } + await renderApplication(bootstrap, { + document: indexHtml, + url: '/error', + }); } runTest(); diff --git a/integration/platform-server-zoneless/projects/standalone/src/app/app.config.ts b/integration/platform-server-zoneless/projects/standalone/src/app/app.config.ts index b540528ec02..82df12263ad 100644 --- a/integration/platform-server-zoneless/projects/standalone/src/app/app.config.ts +++ b/integration/platform-server-zoneless/projects/standalone/src/app/app.config.ts @@ -1,5 +1,5 @@ import {provideHttpClient} from '@angular/common/http'; -import {ApplicationConfig, provideZonelessChangeDetection} from '@angular/core'; +import {ApplicationConfig, provideExperimentalZonelessChangeDetection} from '@angular/core'; import {provideClientHydration, withIncrementalHydration} from '@angular/platform-browser'; import {provideRouter} from '@angular/router'; @@ -7,7 +7,7 @@ import {routes} from './app.routes'; export const appConfig: ApplicationConfig = { providers: [ - provideZonelessChangeDetection(), + provideExperimentalZonelessChangeDetection(), provideRouter(routes), provideClientHydration(withIncrementalHydration()), provideHttpClient(), diff --git a/integration/platform-server-zoneless/projects/standalone/src/app/app.routes.ts b/integration/platform-server-zoneless/projects/standalone/src/app/app.routes.ts index 72c90fb0e7d..390276b2b35 100644 --- a/integration/platform-server-zoneless/projects/standalone/src/app/app.routes.ts +++ b/integration/platform-server-zoneless/projects/standalone/src/app/app.routes.ts @@ -29,11 +29,6 @@ export const routes: Routes = [ { path: 'error', component: HelloWorldComponent, - resolve: { - 'id': () => { - throw new Error('Error in resolver.'); - }, - }, }, { path: 'defer', diff --git a/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts b/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts index 579227ddcdc..99fd4a54b0c 100644 --- a/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts +++ b/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts @@ -7,23 +7,25 @@ */ import {HttpClient} from '@angular/common/http'; -import {Component, OnInit} from '@angular/core'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, inject, OnInit} from '@angular/core'; @Component({ selector: 'transfer-state-http', standalone: true, template: `
{{ responseOne }}
`, providers: [HttpClient], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class TransferStateOnInitComponent implements OnInit { responseOne: string = ''; - - constructor(private readonly httpClient: HttpClient) {} + private readonly httpClient: HttpClient = inject(HttpClient); + private readonly cdr: ChangeDetectorRef = inject(ChangeDetectorRef); ngOnInit(): void { // Test that HTTP cache works when HTTP call is made in a lifecycle hook. this.httpClient.get('http://localhost:4206/api').subscribe((response) => { this.responseOne = response.data; + this.cdr.markForCheck(); }); } } diff --git a/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts b/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts index 88712e7dc8d..c4804ceeccb 100644 --- a/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts +++ b/integration/platform-server-zoneless/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts @@ -7,7 +7,7 @@ */ import {HttpClient} from '@angular/common/http'; -import {Component, OnInit} from '@angular/core'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, inject, OnInit} from '@angular/core'; @Component({ selector: 'transfer-state-http', @@ -17,15 +17,19 @@ import {Component, OnInit} from '@angular/core';
{{ responseTwo }}
`, providers: [HttpClient], + changeDetection: ChangeDetectionStrategy.OnPush, }) export class TransferStateComponent implements OnInit { responseOne: string = ''; responseTwo: string = ''; + private readonly httpClient: HttpClient = inject(HttpClient); + private readonly cdr: ChangeDetectorRef = inject(ChangeDetectorRef); - constructor(private readonly httpClient: HttpClient) { + constructor() { // Test that HTTP cache works when HTTP call is made in the constructor. this.httpClient.get('http://localhost:4206/api').subscribe((response) => { this.responseOne = response.data; + this.cdr.markForCheck(); }); } @@ -33,6 +37,7 @@ export class TransferStateComponent implements OnInit { // Test that HTTP cache works when HTTP call is made in a lifecycle hook. this.httpClient.get('/api-2').subscribe((response) => { this.responseTwo = response.data; + this.cdr.markForCheck(); }); } } diff --git a/packages/core/src/defer/triggering.ts b/packages/core/src/defer/triggering.ts index 6d01892d387..e3ea7b8032f 100644 --- a/packages/core/src/defer/triggering.ts +++ b/packages/core/src/defer/triggering.ts @@ -21,7 +21,7 @@ import { getParentBlockHydrationQueue, isIncrementalHydrationEnabled, } from '../hydration/utils'; -import {PendingTasksInternal} from '../pending_tasks'; +import {PendingTasks, PendingTasksInternal} from '../pending_tasks'; import {assertLContainer} from '../render3/assert'; import {getComponentDef, getDirectiveDef, getPipeDef} from '../render3/def_getters'; import {getTemplateLocationDetails} from '../render3/instructions/element_validation'; @@ -205,8 +205,7 @@ export function triggerResourceLoading( } // Indicate that an application is not stable and has a pending task. - const pendingTasks = injector.get(PendingTasksInternal); - const taskId = pendingTasks.add(); + const removeTask = injector.get(PendingTasks).add(); // The `dependenciesFn` might be `null` when all dependencies within // a given defer block were eagerly referenced elsewhere in a file, @@ -215,7 +214,7 @@ export function triggerResourceLoading( tDetails.loadingPromise = Promise.resolve().then(() => { tDetails.loadingPromise = null; tDetails.loadingState = DeferDependenciesLoadingState.COMPLETE; - pendingTasks.remove(taskId); + removeTask(); }); return tDetails.loadingPromise; } @@ -244,11 +243,6 @@ export function triggerResourceLoading( } } - // Loading is completed, we no longer need the loading Promise - // and a pending task should also be removed. - tDetails.loadingPromise = null; - pendingTasks.remove(taskId); - if (failed) { tDetails.loadingState = DeferDependenciesLoadingState.FAILED; @@ -288,7 +282,13 @@ export function triggerResourceLoading( } } }); - return tDetails.loadingPromise; + + return tDetails.loadingPromise.finally(() => { + // Loading is completed, we no longer need the loading Promise + // and a pending task should also be removed. + tDetails.loadingPromise = null; + removeTask(); + }); } /** diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index c88d832d3fd..ddc108ec7bf 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -137,6 +137,7 @@ "PREFETCH_TRIGGER_CLEANUP_FNS", "PRESERVE_HOST_CONTENT", "PRESERVE_HOST_CONTENT_DEFAULT", + "PendingTasks", "PendingTasksInternal", "R3Injector", "REACTIVE_LVIEW_CONSUMER_NODE",