Skip to content

Commit

Permalink
fix(core): wait for HTTP in ngOnInit correctly before server render (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
alan-agius4 authored and alxhub committed Jun 6, 2023
1 parent a01ecf3 commit edceb48
Show file tree
Hide file tree
Showing 17 changed files with 315 additions and 6 deletions.
8 changes: 4 additions & 4 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -2,7 +2,7 @@
"cli-hello-world": {
"uncompressed": {
"runtime": 908,
"main": 129295,
"main": 134468,
"polyfills": 33792
}
},
Expand All @@ -24,14 +24,14 @@
"forms": {
"uncompressed": {
"runtime": 888,
"main": 160778,
"main": 166256,
"polyfills": 33772
}
},
"animations": {
"uncompressed": {
"runtime": 898,
"main": 159461,
"main": 164757,
"polyfills": 33782
}
},
Expand All @@ -56,4 +56,4 @@
"polyfills": 33802
}
}
}
}
@@ -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([]);
});
});
Expand Up @@ -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({
Expand Down
@@ -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: ` <div class="one">{{ responseOne }}</div> `,
})
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<any>('http://localhost:4206/api').subscribe((response) => {
this.responseOne = response.data;
});
}
}
@@ -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 {}
Expand Up @@ -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
),
},
];
@@ -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: ` <div class="one">{{ responseOne }}</div> `,
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<any>('http://localhost:4206/api').subscribe((response) => {
this.responseOne = response.data;
});
}
}
4 changes: 2 additions & 2 deletions packages/core/src/application_ref.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -847,7 +847,7 @@ export class ApplicationRef {
public readonly isStable: Observable<boolean> =
inject(InitialRenderPendingTasks)
.hasPendingTasks.pipe(
mergeMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
switchMap(hasPendingTasks => hasPendingTasks ? of(false) : this.zoneIsStable),
distinctUntilChanged(),
share(),
);
Expand Down
Expand Up @@ -500,6 +500,12 @@
{
"name": "Subscription"
},
{
"name": "SwitchMapOperator"
},
{
"name": "SwitchMapSubscriber"
},
{
"name": "TESTABILITY"
},
Expand Down Expand Up @@ -863,6 +869,9 @@
{
"name": "forwardRef"
},
{
"name": "from"
},
{
"name": "fromArray"
},
Expand Down Expand Up @@ -1046,6 +1055,9 @@
{
"name": "injectableDefOrInjectorDefFactory"
},
{
"name": "innerSubscribe"
},
{
"name": "insertBloom"
},
Expand Down Expand Up @@ -1187,6 +1199,9 @@
{
"name": "makeTimingAst"
},
{
"name": "map"
},
{
"name": "markAsComponentHost"
},
Expand All @@ -1199,6 +1214,9 @@
{
"name": "maybeWrapInNotSelector"
},
{
"name": "mergeAll"
},
{
"name": "mergeHostAttribute"
},
Expand Down Expand Up @@ -1421,6 +1439,9 @@
{
"name": "subscribeToArray"
},
{
"name": "switchMap"
},
{
"name": "throwProviderNotFoundError"
},
Expand Down
21 changes: 21 additions & 0 deletions packages/core/test/bundling/animations/bundle.golden_symbols.json
Expand Up @@ -542,6 +542,12 @@
{
"name": "Subscription"
},
{
"name": "SwitchMapOperator"
},
{
"name": "SwitchMapSubscriber"
},
{
"name": "TESTABILITY"
},
Expand Down Expand Up @@ -926,6 +932,9 @@
{
"name": "forwardRef"
},
{
"name": "from"
},
{
"name": "fromArray"
},
Expand Down Expand Up @@ -1112,6 +1121,9 @@
{
"name": "injectableDefOrInjectorDefFactory"
},
{
"name": "innerSubscribe"
},
{
"name": "insertBloom"
},
Expand Down Expand Up @@ -1253,6 +1265,9 @@
{
"name": "makeTimingAst"
},
{
"name": "map"
},
{
"name": "markAsComponentHost"
},
Expand All @@ -1265,6 +1280,9 @@
{
"name": "maybeWrapInNotSelector"
},
{
"name": "mergeAll"
},
{
"name": "mergeHostAttribute"
},
Expand Down Expand Up @@ -1496,6 +1514,9 @@
{
"name": "subscribeToArray"
},
{
"name": "switchMap"
},
{
"name": "throwProviderNotFoundError"
},
Expand Down
Expand Up @@ -407,6 +407,12 @@
{
"name": "Subscription"
},
{
"name": "SwitchMapOperator"
},
{
"name": "SwitchMapSubscriber"
},
{
"name": "TESTABILITY"
},
Expand Down Expand Up @@ -701,6 +707,9 @@
{
"name": "forwardRef"
},
{
"name": "from"
},
{
"name": "fromArray"
},
Expand Down Expand Up @@ -878,6 +887,9 @@
{
"name": "injectableDefOrInjectorDefFactory"
},
{
"name": "innerSubscribe"
},
{
"name": "insertBloom"
},
Expand Down Expand Up @@ -992,6 +1004,9 @@
{
"name": "makeRecord"
},
{
"name": "map"
},
{
"name": "markAsComponentHost"
},
Expand All @@ -1004,6 +1019,9 @@
{
"name": "maybeWrapInNotSelector"
},
{
"name": "mergeAll"
},
{
"name": "mergeHostAttribute"
},
Expand Down Expand Up @@ -1187,6 +1205,9 @@
{
"name": "subscribeToArray"
},
{
"name": "switchMap"
},
{
"name": "throwProviderNotFoundError"
},
Expand Down

0 comments on commit edceb48

Please sign in to comment.