From 2fef57232fadd1be9321a3ce915bc30762d992da Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Mon, 5 Jun 2023 09:53:16 +0000 Subject: [PATCH] fix(core): wait for HTTP in `ngOnInit` correctly before server render Previously, with `mergeMap` we did not cancel previous subscriptions to zoneIsStable which caused the application to be stablized before hand. Closes: #50562 --- .../http-transferstate-lazy-on-init-spec.ts | 44 +++++++++++++++++++ .../ngmodule/src/app/app-routing.module.ts | 7 +++ ...tp-transferstate-lazy-on-init.component.ts | 27 ++++++++++++ .../http-transferstate-lazy-on-init.module.ts | 18 ++++++++ .../projects/standalone/src/app/app.routes.ts | 7 +++ .../http-transfer-state-on-init.component.ts | 29 ++++++++++++ packages/core/src/application_ref.ts | 4 +- .../bundle.golden_symbols.json | 21 +++++++++ .../animations/bundle.golden_symbols.json | 21 +++++++++ .../cyclic_import/bundle.golden_symbols.json | 21 +++++++++ .../forms_reactive/bundle.golden_symbols.json | 15 +++++++ .../bundle.golden_symbols.json | 15 +++++++ .../hello_world/bundle.golden_symbols.json | 21 +++++++++ .../hydration/bundle.golden_symbols.json | 21 +++++++++ .../bundle.golden_symbols.json | 21 +++++++++ .../bundling/todo/bundle.golden_symbols.json | 21 +++++++++ 16 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 integration/platform-server/e2e/src/http-transferstate-lazy-on-init-spec.ts create mode 100644 integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.component.ts create mode 100644 integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module.ts create mode 100644 integration/platform-server/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts diff --git a/integration/platform-server/e2e/src/http-transferstate-lazy-on-init-spec.ts b/integration/platform-server/e2e/src/http-transferstate-lazy-on-init-spec.ts new file mode 100644 index 00000000000000..af7081933d95ca --- /dev/null +++ b/integration/platform-server/e2e/src/http-transferstate-lazy-on-init-spec.ts @@ -0,0 +1,44 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * 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 {browser, by, element} from 'protractor'; +import {bootstrapClientApp, navigateTo, verifyNoBrowserErrors} from './util'; + +describe('Http TransferState Lazy On Init', () => { + beforeEach(async () => { + // Don't wait for Angular since it is not bootstrapped automatically. + await browser.waitForAngularEnabled(false); + + // Load the page without waiting for Angular since it is not bootstrapped automatically. + await navigateTo('http-transferstate-lazy-on-init'); + }); + + afterEach(async () => { + // Make sure there were no client side errors. + await verifyNoBrowserErrors(); + }); + + it('should transfer http state in lazy component', async () => { + // Test the contents from the server. + expect(await element(by.css('div.one')).getText()).toBe('API 1 response'); + + // Bootstrap the client side app and retest the contents + await bootstrapClientApp(); + expect(await element(by.css('div.one')).getText()).toBe('API 1 response'); + + // Validate that there were no HTTP calls to '/api'. + const requests = await browser.executeScript(() => { + return performance.getEntriesByType('resource'); + }); + const apiRequests = (requests as {name: string}[]) + .filter(({name}) => name.includes('/api')) + .map(({name}) => name); + + expect(apiRequests).toEqual([]); + }); +}); diff --git a/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts b/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts index 701e7bc35a78e0..15b321b453caaa 100644 --- a/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts +++ b/integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts @@ -19,6 +19,13 @@ const routes: Routes = [ (m) => m.HttpTransferStateModule ), }, + { + path: 'http-transferstate-lazy-on-init', + loadChildren: () => + import('./http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module').then( + (m) => m.HttpTransferStateOnInitModule + ), + }, ]; @NgModule({ diff --git a/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.component.ts b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.component.ts new file mode 100644 index 00000000000000..be88110e9192a2 --- /dev/null +++ b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.component.ts @@ -0,0 +1,27 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * 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 {HttpClient} from '@angular/common/http'; +import {Component, OnInit} from '@angular/core'; + +@Component({ + selector: 'transfer-state-http-on-init', + template: `
{{ responseOne }}
`, +}) +export class TransferStateComponentOnInit implements OnInit { + responseOne: string = ''; + + constructor(private readonly httpClient: HttpClient) {} + + 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; + }); + } +} diff --git a/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module.ts b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module.ts new file mode 100644 index 00000000000000..46fa2fd3ca0cc0 --- /dev/null +++ b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module.ts @@ -0,0 +1,18 @@ +import {CommonModule} from '@angular/common'; +import {HttpClientModule} from '@angular/common/http'; +import {NgModule} from '@angular/core'; +import {RouterModule, Routes} from '@angular/router'; +import {TransferStateComponentOnInit} from './http-transferstate-lazy-on-init.component'; + +const routes: Routes = [ + { + path: '', + component: TransferStateComponentOnInit, + }, +]; + +@NgModule({ + imports: [RouterModule.forChild(routes), HttpClientModule, CommonModule], + declarations: [TransferStateComponentOnInit], +}) +export class HttpTransferStateOnInitModule {} diff --git a/integration/platform-server/projects/standalone/src/app/app.routes.ts b/integration/platform-server/projects/standalone/src/app/app.routes.ts index 851ce59d3736a6..697c7ecb7301ff 100644 --- a/integration/platform-server/projects/standalone/src/app/app.routes.ts +++ b/integration/platform-server/projects/standalone/src/app/app.routes.ts @@ -18,4 +18,11 @@ export const routes: Routes = [ (c) => c.TransferStateComponent ), }, + { + path: 'http-transferstate-lazy-on-init', + loadComponent: () => + import('./http-transferstate-lazy-on-init/http-transfer-state-on-init.component').then( + (c) => c.TransferStateOnInitComponent + ), + }, ]; diff --git a/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts b/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts new file mode 100644 index 00000000000000..6801b8c7708a75 --- /dev/null +++ b/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy-on-init/http-transfer-state-on-init.component.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * 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 {HttpClient} from '@angular/common/http'; +import {Component, OnInit} from '@angular/core'; + +@Component({ + selector: 'transfer-state-http', + standalone: true, + template: `
{{ responseOne }}
`, + providers: [HttpClient], +}) +export class TransferStateOnInitComponent implements OnInit { + responseOne: string = ''; + + constructor(private readonly httpClient: HttpClient) {} + + 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; + }); + } +} diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index ec9b181f648c7f..7ee09d94ac61cf 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -9,7 +9,7 @@ import './util/ng_jit_mode'; import {Observable, of, Subscription} from 'rxjs'; -import {distinctUntilChanged, mergeMap, share} from 'rxjs/operators'; +import {distinctUntilChanged, share, switchMap} from 'rxjs/operators'; import {ApplicationInitStatus} from './application_init'; import {PLATFORM_INITIALIZER} from './application_tokens'; @@ -847,7 +847,7 @@ export class ApplicationRef { public readonly isStable: Observable = inject(InitialRenderPendingTasks) .hasPendingTasks.pipe( - mergeMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable), + switchMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable), distinctUntilChanged(), share(), ); 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 5f3fa35a68c9e1..211416c63f3099 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -500,6 +500,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -863,6 +869,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -1046,6 +1055,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1187,6 +1199,9 @@ { "name": "makeTimingAst" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, @@ -1199,6 +1214,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1421,6 +1439,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index 32a8d551784409..8cb5432ebec0b9 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -542,6 +542,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -926,6 +932,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -1112,6 +1121,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1253,6 +1265,9 @@ { "name": "makeTimingAst" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, @@ -1265,6 +1280,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1496,6 +1514,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, 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 fd5e9f963d942e..de7c9f68b40675 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -407,6 +407,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -701,6 +707,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -878,6 +887,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -992,6 +1004,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, @@ -1004,6 +1019,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1187,6 +1205,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, 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 4e607d66d04cd3..2004bd6a7f5584 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -548,6 +548,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -1211,6 +1217,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1391,6 +1400,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeErrors" }, @@ -1652,6 +1664,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, 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 fe9d90b885a21b..bc32c98cd0281d 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 @@ -536,6 +536,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -1175,6 +1181,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1349,6 +1358,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeErrors" }, @@ -1628,6 +1640,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, 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 19bf5ce51b57ad..cd39e0e2848c9c 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -314,6 +314,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -548,6 +554,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -698,6 +707,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -779,6 +791,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, @@ -788,6 +803,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -935,6 +953,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index b69ebc9e4c68fd..2d01939370aed1 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -458,6 +458,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -758,6 +764,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -938,6 +947,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1058,6 +1070,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, @@ -1067,6 +1082,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1247,6 +1265,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwIfEmpty" }, 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 cd7ab5a673bdd5..15ec1e64503035 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -374,6 +374,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -626,6 +632,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -782,6 +791,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -872,6 +884,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markViewDirty" }, @@ -881,6 +896,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1037,6 +1055,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 08696622068d06..cda195c430fb46 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -437,6 +437,12 @@ { "name": "Subscription" }, + { + "name": "SwitchMapOperator" + }, + { + "name": "SwitchMapSubscriber" + }, { "name": "TESTABILITY" }, @@ -824,6 +830,9 @@ { "name": "forwardRef" }, + { + "name": "from" + }, { "name": "fromArray" }, @@ -1049,6 +1058,9 @@ { "name": "injectableDefOrInjectorDefFactory" }, + { + "name": "innerSubscribe" + }, { "name": "insertBloom" }, @@ -1187,6 +1199,9 @@ { "name": "makeRecord" }, + { + "name": "map" + }, { "name": "markAsComponentHost" }, @@ -1202,6 +1217,9 @@ { "name": "maybeWrapInNotSelector" }, + { + "name": "mergeAll" + }, { "name": "mergeHostAttribute" }, @@ -1412,6 +1430,9 @@ { "name": "subscribeToArray" }, + { + "name": "switchMap" + }, { "name": "throwProviderNotFoundError" },