Skip to content

Commit c992109

Browse files
alan-agius4alxhub
authored andcommitted
fix(core): wait for HTTP in ngOnInit correctly before server render (#50573)
Previously, with `mergeMap` we did not cancel previous subscriptions to zoneIsStable which caused the application to be stablized before hand. Closes: #50562 PR Close #50573
1 parent 5cfb6ca commit c992109

File tree

17 files changed

+315
-6
lines changed

17 files changed

+315
-6
lines changed

goldens/size-tracking/integration-payloads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"cli-hello-world": {
33
"uncompressed": {
44
"runtime": 908,
5-
"main": 129295,
5+
"main": 134468,
66
"polyfills": 33792
77
}
88
},
@@ -24,14 +24,14 @@
2424
"forms": {
2525
"uncompressed": {
2626
"runtime": 888,
27-
"main": 160778,
27+
"main": 166256,
2828
"polyfills": 33772
2929
}
3030
},
3131
"animations": {
3232
"uncompressed": {
3333
"runtime": 898,
34-
"main": 159461,
34+
"main": 164757,
3535
"polyfills": 33782
3636
}
3737
},
@@ -56,4 +56,4 @@
5656
"polyfills": 33802
5757
}
5858
}
59-
}
59+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {browser, by, element} from 'protractor';
10+
import {bootstrapClientApp, navigateTo, verifyNoBrowserErrors} from './util';
11+
12+
describe('Http TransferState Lazy On Init', () => {
13+
beforeEach(async () => {
14+
// Don't wait for Angular since it is not bootstrapped automatically.
15+
await browser.waitForAngularEnabled(false);
16+
17+
// Load the page without waiting for Angular since it is not bootstrapped automatically.
18+
await navigateTo('http-transferstate-lazy-on-init');
19+
});
20+
21+
afterEach(async () => {
22+
// Make sure there were no client side errors.
23+
await verifyNoBrowserErrors();
24+
});
25+
26+
it('should transfer http state in lazy component', async () => {
27+
// Test the contents from the server.
28+
expect(await element(by.css('div.one')).getText()).toBe('API 1 response');
29+
30+
// Bootstrap the client side app and retest the contents
31+
await bootstrapClientApp();
32+
expect(await element(by.css('div.one')).getText()).toBe('API 1 response');
33+
34+
// Validate that there were no HTTP calls to '/api'.
35+
const requests = await browser.executeScript(() => {
36+
return performance.getEntriesByType('resource');
37+
});
38+
const apiRequests = (requests as {name: string}[])
39+
.filter(({name}) => name.includes('/api'))
40+
.map(({name}) => name);
41+
42+
expect(apiRequests).toEqual([]);
43+
});
44+
});

integration/platform-server/projects/ngmodule/src/app/app-routing.module.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ const routes: Routes = [
1919
(m) => m.HttpTransferStateModule
2020
),
2121
},
22+
{
23+
path: 'http-transferstate-lazy-on-init',
24+
loadChildren: () =>
25+
import('./http-transferstate-lazy-on-init/http-transferstate-lazy-on-init.module').then(
26+
(m) => m.HttpTransferStateOnInitModule
27+
),
28+
},
2229
];
2330

2431
@NgModule({
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {HttpClient} from '@angular/common/http';
10+
import {Component, OnInit} from '@angular/core';
11+
12+
@Component({
13+
selector: 'transfer-state-http-on-init',
14+
template: ` <div class="one">{{ responseOne }}</div> `,
15+
})
16+
export class TransferStateComponentOnInit implements OnInit {
17+
responseOne: string = '';
18+
19+
constructor(private readonly httpClient: HttpClient) {}
20+
21+
ngOnInit(): void {
22+
// Test that HTTP cache works when HTTP call is made in a lifecycle hook.
23+
this.httpClient.get<any>('http://localhost:4206/api').subscribe((response) => {
24+
this.responseOne = response.data;
25+
});
26+
}
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import {CommonModule} from '@angular/common';
2+
import {HttpClientModule} from '@angular/common/http';
3+
import {NgModule} from '@angular/core';
4+
import {RouterModule, Routes} from '@angular/router';
5+
import {TransferStateComponentOnInit} from './http-transferstate-lazy-on-init.component';
6+
7+
const routes: Routes = [
8+
{
9+
path: '',
10+
component: TransferStateComponentOnInit,
11+
},
12+
];
13+
14+
@NgModule({
15+
imports: [RouterModule.forChild(routes), HttpClientModule, CommonModule],
16+
declarations: [TransferStateComponentOnInit],
17+
})
18+
export class HttpTransferStateOnInitModule {}

integration/platform-server/projects/standalone/src/app/app.routes.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,11 @@ export const routes: Routes = [
1818
(c) => c.TransferStateComponent
1919
),
2020
},
21+
{
22+
path: 'http-transferstate-lazy-on-init',
23+
loadComponent: () =>
24+
import('./http-transferstate-lazy-on-init/http-transfer-state-on-init.component').then(
25+
(c) => c.TransferStateOnInitComponent
26+
),
27+
},
2128
];
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {HttpClient} from '@angular/common/http';
10+
import {Component, OnInit} from '@angular/core';
11+
12+
@Component({
13+
selector: 'transfer-state-http',
14+
standalone: true,
15+
template: ` <div class="one">{{ responseOne }}</div> `,
16+
providers: [HttpClient],
17+
})
18+
export class TransferStateOnInitComponent implements OnInit {
19+
responseOne: string = '';
20+
21+
constructor(private readonly httpClient: HttpClient) {}
22+
23+
ngOnInit(): void {
24+
// Test that HTTP cache works when HTTP call is made in a lifecycle hook.
25+
this.httpClient.get<any>('http://localhost:4206/api').subscribe((response) => {
26+
this.responseOne = response.data;
27+
});
28+
}
29+
}

packages/core/src/application_ref.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import './util/ng_jit_mode';
1010

1111
import {Observable, of, Subscription} from 'rxjs';
12-
import {distinctUntilChanged, mergeMap, share} from 'rxjs/operators';
12+
import {distinctUntilChanged, share, switchMap} from 'rxjs/operators';
1313

1414
import {ApplicationInitStatus} from './application_init';
1515
import {PLATFORM_INITIALIZER} from './application_tokens';
@@ -847,7 +847,7 @@ export class ApplicationRef {
847847
public readonly isStable: Observable<boolean> =
848848
inject(InitialRenderPendingTasks)
849849
.hasPendingTasks.pipe(
850-
mergeMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
850+
switchMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
851851
distinctUntilChanged(),
852852
share(),
853853
);

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,12 @@
500500
{
501501
"name": "Subscription"
502502
},
503+
{
504+
"name": "SwitchMapOperator"
505+
},
506+
{
507+
"name": "SwitchMapSubscriber"
508+
},
503509
{
504510
"name": "TESTABILITY"
505511
},
@@ -851,6 +857,9 @@
851857
{
852858
"name": "forwardRef"
853859
},
860+
{
861+
"name": "from"
862+
},
854863
{
855864
"name": "fromArray"
856865
},
@@ -1034,6 +1043,9 @@
10341043
{
10351044
"name": "injectableDefOrInjectorDefFactory"
10361045
},
1046+
{
1047+
"name": "innerSubscribe"
1048+
},
10371049
{
10381050
"name": "insertBloom"
10391051
},
@@ -1175,6 +1187,9 @@
11751187
{
11761188
"name": "makeTimingAst"
11771189
},
1190+
{
1191+
"name": "map"
1192+
},
11781193
{
11791194
"name": "markAsComponentHost"
11801195
},
@@ -1187,6 +1202,9 @@
11871202
{
11881203
"name": "maybeWrapInNotSelector"
11891204
},
1205+
{
1206+
"name": "mergeAll"
1207+
},
11901208
{
11911209
"name": "mergeHostAttribute"
11921210
},
@@ -1415,6 +1433,9 @@
14151433
{
14161434
"name": "subscribeToArray"
14171435
},
1436+
{
1437+
"name": "switchMap"
1438+
},
14181439
{
14191440
"name": "throwProviderNotFoundError"
14201441
},

packages/core/test/bundling/animations/bundle.golden_symbols.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,12 @@
542542
{
543543
"name": "Subscription"
544544
},
545+
{
546+
"name": "SwitchMapOperator"
547+
},
548+
{
549+
"name": "SwitchMapSubscriber"
550+
},
545551
{
546552
"name": "TESTABILITY"
547553
},
@@ -914,6 +920,9 @@
914920
{
915921
"name": "forwardRef"
916922
},
923+
{
924+
"name": "from"
925+
},
917926
{
918927
"name": "fromArray"
919928
},
@@ -1100,6 +1109,9 @@
11001109
{
11011110
"name": "injectableDefOrInjectorDefFactory"
11021111
},
1112+
{
1113+
"name": "innerSubscribe"
1114+
},
11031115
{
11041116
"name": "insertBloom"
11051117
},
@@ -1241,6 +1253,9 @@
12411253
{
12421254
"name": "makeTimingAst"
12431255
},
1256+
{
1257+
"name": "map"
1258+
},
12441259
{
12451260
"name": "markAsComponentHost"
12461261
},
@@ -1253,6 +1268,9 @@
12531268
{
12541269
"name": "maybeWrapInNotSelector"
12551270
},
1271+
{
1272+
"name": "mergeAll"
1273+
},
12561274
{
12571275
"name": "mergeHostAttribute"
12581276
},
@@ -1490,6 +1508,9 @@
14901508
{
14911509
"name": "subscribeToArray"
14921510
},
1511+
{
1512+
"name": "switchMap"
1513+
},
14931514
{
14941515
"name": "throwProviderNotFoundError"
14951516
},

0 commit comments

Comments
 (0)