diff --git a/goldens/public-api/common/http/index.md b/goldens/public-api/common/http/index.md index 1cc31ac6928..c08fe630e94 100644 --- a/goldens/public-api/common/http/index.md +++ b/goldens/public-api/common/http/index.md @@ -10,6 +10,7 @@ import * as i0 from '@angular/core'; import { InjectionToken } from '@angular/core'; import { ModuleWithProviders } from '@angular/core'; import { Observable } from 'rxjs'; +import { OnDestroy } from '@angular/core'; import { Provider } from '@angular/core'; import { XhrFactory } from '@angular/common'; @@ -2128,10 +2129,12 @@ export interface HttpUserEvent { } // @public -export class HttpXhrBackend implements HttpBackend { +export class HttpXhrBackend implements HttpBackend, OnDestroy { constructor(xhrFactory: XhrFactory); handle(req: HttpRequest): Observable>; // (undocumented) + ngOnDestroy(): void; + // (undocumented) static ɵfac: i0.ɵɵFactoryDeclaration; // (undocumented) static ɵprov: i0.ɵɵInjectableDeclaration; diff --git a/integration/platform-server/e2e/http-transferstate-lazy-spec.ts b/integration/platform-server/e2e/http-transferstate-lazy-spec.ts new file mode 100644 index 00000000000..098f3326e74 --- /dev/null +++ b/integration/platform-server/e2e/http-transferstate-lazy-spec.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 {browser, by, element} from 'protractor'; + +import {verifyNoBrowserErrors} from './util'; + +describe('Http TransferState Lazy', function() { + it('should transfer http state in lazy component', function() { + // Load the page without waiting for Angular since it is not bootstrapped automatically. + browser.driver.get(browser.baseUrl + 'http-transferstate-lazy'); + + // Test the contents from the server. + const serverDiv = browser.driver.findElement(by.css('div')); + expect(serverDiv.getText()).toBe('API response'); + + // Bootstrap the client side app and retest the contents + browser.executeScript('doBootstrap()'); + expect(element(by.css('div')).getText()).toBe('API response'); + + // Make sure there were no client side errors. + verifyNoBrowserErrors(); + }); +}); diff --git a/integration/platform-server/package.json b/integration/platform-server/package.json index da878efade9..4d6a374ba37 100644 --- a/integration/platform-server/package.json +++ b/integration/platform-server/package.json @@ -16,6 +16,7 @@ "@angular/platform-browser": "file:../../dist/packages-dist/platform-browser", "@angular/platform-browser-dynamic": "file:../../dist/packages-dist/platform-browser-dynamic", "@angular/platform-server": "file:../../dist/packages-dist/platform-server", + "@angular/router": "file:../../dist/packages-dist/router", "express": "4.16.4", "rxjs": "file:../../node_modules/rxjs", "typescript": "file:../../node_modules/typescript", diff --git a/integration/platform-server/src/http-transferstate-lazy/app.component.ts b/integration/platform-server/src/http-transferstate-lazy/app.component.ts new file mode 100644 index 00000000000..40c0ecf54ec --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/app.component.ts @@ -0,0 +1,8 @@ +import { Component } from '@angular/core'; + +@Component({ + selector: 'app-root', + template: '', +}) +export class AppComponent { +} diff --git a/integration/platform-server/src/http-transferstate-lazy/app.server.ts b/integration/platform-server/src/http-transferstate-lazy/app.server.ts new file mode 100644 index 00000000000..cb8c3c1ac5c --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/app.server.ts @@ -0,0 +1,20 @@ +/** + * @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 {NgModule} from '@angular/core'; +import {ServerModule} from '@angular/platform-server'; + +import {HttpLazyTransferStateModule} from './app'; +import {AppComponent} from './app.component'; + +@NgModule({ + bootstrap: [AppComponent], + imports: [HttpLazyTransferStateModule, ServerModule], +}) +export class HttpLazyTransferStateServerModule { +} diff --git a/integration/platform-server/src/http-transferstate-lazy/app.ts b/integration/platform-server/src/http-transferstate-lazy/app.ts new file mode 100644 index 00000000000..9ad06e0b43e --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/app.ts @@ -0,0 +1,30 @@ +/** + * @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 {NgModule} from '@angular/core'; +import {BrowserModule} from '@angular/platform-browser'; +import {RouterModule, Routes} from '@angular/router'; +import {AppComponent} from './app.component'; + +const routes: Routes = [ + { + path: '', + loadChildren: () => import('./transfer-state.module').then((m) => m.TransferStateModule), + }, +]; + +@NgModule({ + declarations: [AppComponent], + bootstrap: [AppComponent], + imports: [ + BrowserModule, + RouterModule.forRoot(routes), + ], +}) +export class HttpLazyTransferStateModule { +} diff --git a/integration/platform-server/src/http-transferstate-lazy/client.ts b/integration/platform-server/src/http-transferstate-lazy/client.ts new file mode 100644 index 00000000000..1b29048a6c1 --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/client.ts @@ -0,0 +1,16 @@ +/** + * @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 'zone.js/bundles/zone.umd'; + +import {platformBrowser} from '@angular/platform-browser'; +import {HttpLazyTransferStateModule} from './app'; + +window['doBootstrap'] = function() { + platformBrowser().bootstrapModule(HttpLazyTransferStateModule); +}; diff --git a/integration/platform-server/src/http-transferstate-lazy/index.html b/integration/platform-server/src/http-transferstate-lazy/index.html new file mode 100644 index 00000000000..c83f90a5d25 --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/index.html @@ -0,0 +1,11 @@ + + + + + Hello World + + + + + + diff --git a/integration/platform-server/src/http-transferstate-lazy/transfer-state.component.ts b/integration/platform-server/src/http-transferstate-lazy/transfer-state.component.ts new file mode 100644 index 00000000000..a62550c84f1 --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/transfer-state.component.ts @@ -0,0 +1,41 @@ +/** + * @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 {isPlatformServer} from '@angular/common'; +import {HttpClient} from '@angular/common/http'; +import {Component, Inject, PLATFORM_ID} from '@angular/core'; +import {TransferState, makeStateKey} from '@angular/platform-browser'; + +const httpCacheKey = makeStateKey('http'); + +@Component({ + selector: 'transfer-state-app-http', + template: ` +
{{ response }}
+ `, +}) +export class TransferStateComponent { + response: string = ''; + + constructor( + @Inject(PLATFORM_ID) private platformId: {}, + private readonly httpClient: HttpClient, + private readonly transferState: TransferState + ) {} + + ngOnInit() { + if (isPlatformServer(this.platformId)) { + this.httpClient.get(`http://localhost:4206/api`).subscribe((response) => { + this.transferState.set(httpCacheKey, response.data); + this.response = response.data; + }); + } else { + this.response = this.transferState.get(httpCacheKey, ''); + } + } +} diff --git a/integration/platform-server/src/http-transferstate-lazy/transfer-state.module.ts b/integration/platform-server/src/http-transferstate-lazy/transfer-state.module.ts new file mode 100644 index 00000000000..4097c21371c --- /dev/null +++ b/integration/platform-server/src/http-transferstate-lazy/transfer-state.module.ts @@ -0,0 +1,20 @@ +import { CommonModule } from '@angular/common'; +import { HttpClientModule } from '@angular/common/http'; +import { NgModule } from '@angular/core'; +import { RouterModule, Routes } from '@angular/router'; +import { TransferStateComponent } from './transfer-state.component'; + +const routes: Routes = [ + { + path: '', + component: TransferStateComponent, + }, +]; + +@NgModule({ + imports: [RouterModule.forChild(routes), HttpClientModule, CommonModule], + declarations: [TransferStateComponent], + exports: [TransferStateComponent], +}) +export class TransferStateModule { +} diff --git a/integration/platform-server/src/server.ts b/integration/platform-server/src/server.ts index 41f84499902..ab439168369 100644 --- a/integration/platform-server/src/server.ts +++ b/integration/platform-server/src/server.ts @@ -18,6 +18,9 @@ const {default: helloworld} = require('raw-loader!./helloworld/index.html'); import {TransferStateServerModule} from './transferstate/app.server'; const {default: transferstate} = require('raw-loader!./transferstate/index.html'); +import {HttpLazyTransferStateServerModule} from './http-transferstate-lazy/app.server'; +const {default: httptransferstatelazy} = require('raw-loader!./http-transferstate-lazy/index.html'); + const app = express(); function render(moduleType: Type, html: string) { @@ -25,7 +28,9 @@ function render(moduleType: Type, html: string) { renderModule(moduleType, { document: html, url: req.url, - }).then((response) => { res.send(response); }); + }).then((response) => { + res.send(response); + }); }; } @@ -35,10 +40,20 @@ enableProdMode(); app.use('/webpack-out', express.static('webpack-out')); // Keep the browser logs free of errors. -app.get('/favicon.ico', (req, res) => { res.send(''); }); +app.get('/favicon.ico', (req, res) => { + res.send(''); +}); + +// Mock API +app.get('/api', (req, res) => { + res.json({ data: 'API response'}); +}); //-----------ADD YOUR SERVER SIDE RENDERED APP HERE ---------------------- app.get('/helloworld', render(HelloWorldServerModule, helloworld)); app.get('/transferstate', render(TransferStateServerModule, transferstate)); +app.get('/http-transferstate-lazy', render(HttpLazyTransferStateServerModule, httptransferstatelazy)); -app.listen(4206, function() { console.log('Server listening on port 4206!'); }); +app.listen(4206, () => { + console.log('Server listening on port 4206!'); +}); diff --git a/integration/platform-server/webpack.client.config.mjs b/integration/platform-server/webpack.client.config.mjs index 2e1dbf91d06..9331c8f314b 100644 --- a/integration/platform-server/webpack.client.config.mjs +++ b/integration/platform-server/webpack.client.config.mjs @@ -13,6 +13,7 @@ export default { entry: { helloworld: './built/src/helloworld/client.js', transferstate: './built/src/transferstate/client.js', + httptransferstatelazy: './built/src/http-transferstate-lazy/client.js', }, // Allow for better debugging of this integration test. optimization: {minimize: false}, diff --git a/packages/common/http/src/xhr.ts b/packages/common/http/src/xhr.ts index d504edee257..63811d79255 100644 --- a/packages/common/http/src/xhr.ts +++ b/packages/common/http/src/xhr.ts @@ -7,7 +7,7 @@ */ import {XhrFactory} from '@angular/common'; -import {Injectable} from '@angular/core'; +import {Injectable, OnDestroy} from '@angular/core'; import {Observable, Observer} from 'rxjs'; import {HttpBackend} from './backend'; @@ -40,7 +40,9 @@ function getResponseUrl(xhr: any): string|null { * @publicApi */ @Injectable() -export class HttpXhrBackend implements HttpBackend { +export class HttpXhrBackend implements HttpBackend, OnDestroy { + private macroTaskCanceller: VoidFunction|undefined; + constructor(private xhrFactory: XhrFactory) {} /** @@ -293,18 +295,34 @@ export class HttpXhrBackend implements HttpBackend { } } + /** Tear down logic to cancel the backround macrotask. */ + const onLoadStart = () => { + this.macroTaskCanceller ??= createBackgroundMacroTask(); + }; + const onLoadEnd = () => { + this.macroTaskCanceller?.(); + }; + + xhr.addEventListener('loadstart', onLoadStart); + xhr.addEventListener('loadend', onLoadEnd); + // Fire the request, and notify the event stream that it was fired. xhr.send(reqBody!); observer.next({type: HttpEventType.Sent}); - // This is the return from the Observable function, which is the // request cancellation handler. return () => { // On a cancellation, remove all registered event listeners. + xhr.removeEventListener('loadstart', onLoadStart); + xhr.removeEventListener('loadend', onLoadEnd); xhr.removeEventListener('error', onError); xhr.removeEventListener('abort', onError); xhr.removeEventListener('load', onLoad); xhr.removeEventListener('timeout', onError); + + // Cancel the background macrotask. + this.macroTaskCanceller?.(); + if (req.reportProgress) { xhr.removeEventListener('progress', onDownProgress); if (reqBody !== null && xhr.upload) { @@ -319,4 +337,26 @@ export class HttpXhrBackend implements HttpBackend { }; }); } + + ngOnDestroy(): void { + this.macroTaskCanceller?.(); + } +} + +// Cannot use `Number.MAX_VALUE` as it does not fit into a 32-bit signed integer. +const MAX_INT = 2147483647; + +/** + * A method that creates a background macrotask of up to Number.MAX_VALUE. + * + * This is so that Zone.js can intercept HTTP calls, this is important for server rendering, + * as the application is only rendered once the application is stabilized, meaning there are pending + * macro and micro tasks. + * + * @returns a callback method to cancel the macrotask. + */ +function createBackgroundMacroTask(): VoidFunction { + const timeout = setTimeout(() => void 0, MAX_INT); + + return () => clearTimeout(timeout); } diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts index 4b5af4409b5..0d2968652e8 100644 --- a/packages/platform-server/src/http.ts +++ b/packages/platform-server/src/http.ts @@ -8,11 +8,11 @@ import {PlatformLocation, XhrFactory} from '@angular/common'; import {HttpBackend, HttpEvent, HttpHandler, HttpRequest, ɵHttpInterceptorHandler as HttpInterceptorHandler} from '@angular/common/http'; -import {EnvironmentInjector, inject, Injectable, Provider} from '@angular/core'; -import {Observable, Observer, Subscription} from 'rxjs'; +import {EnvironmentInjector, Inject, inject, Injectable, Provider} from '@angular/core'; +import {Observable} from 'rxjs'; import * as xhr2 from 'xhr2'; -import {INITIAL_CONFIG, PlatformConfig} from './tokens'; +import {INITIAL_CONFIG} from './tokens'; // @see https://www.w3.org/Protocols/HTTP/1.1/draft-ietf-http-v11-spec-01#URI-syntax const isAbsoluteUrl = /^[a-zA-Z\-\+.]+:\/\//; @@ -24,93 +24,20 @@ export class ServerXhr implements XhrFactory { } } -export abstract class ZoneMacroTaskWrapper { - wrap(request: S): Observable { - return new Observable((observer: Observer) => { - let task: Task = null!; - let scheduled: boolean = false; - let sub: Subscription|null = null; - let savedResult: any = null; - let savedError: any = null; +// TODO(alanagius): this logic should be re-evauted and moved into `withTransferCache` in +// `@angular/common/http` if still needed. +@Injectable() +export class ServerHttpInterceptorHandler extends HttpInterceptorHandler { + private readonly platformLocation = inject(PlatformLocation); + private readonly config = inject(INITIAL_CONFIG); - const scheduleTask = (_task: Task) => { - task = _task; - scheduled = true; - - const delegate = this.delegate(request); - sub = delegate.subscribe( - res => savedResult = res, - err => { - if (!scheduled) { - throw new Error( - 'An http observable was completed twice. This shouldn\'t happen, please file a bug.'); - } - savedError = err; - scheduled = false; - task.invoke(); - }, - () => { - if (!scheduled) { - throw new Error( - 'An http observable was completed twice. This shouldn\'t happen, please file a bug.'); - } - scheduled = false; - task.invoke(); - }); - }; - - const cancelTask = (_task: Task) => { - if (!scheduled) { - return; - } - scheduled = false; - if (sub) { - sub.unsubscribe(); - sub = null; - } - }; - - const onComplete = () => { - if (savedError !== null) { - observer.error(savedError); - } else { - observer.next(savedResult); - observer.complete(); - } - }; - - // MockBackend for Http is synchronous, which means that if scheduleTask is by - // scheduleMacroTask, the request will hit MockBackend and the response will be - // sent, causing task.invoke() to be called. - const _task = Zone.current.scheduleMacroTask( - 'ZoneMacroTaskWrapper.subscribe', onComplete, {}, () => null, cancelTask); - scheduleTask(_task); - - return () => { - if (scheduled && task) { - task.zone.cancelTask(task); - scheduled = false; - } - if (sub) { - sub.unsubscribe(); - sub = null; - } - }; - }); + constructor() { + const backend = inject(HttpBackend); + const injector = inject(EnvironmentInjector); + super(backend, injector); } - protected abstract delegate(request: S): Observable; -} - -export class ZoneClientBackend extends - ZoneMacroTaskWrapper, HttpEvent> implements HttpBackend { - constructor( - private backend: HttpBackend, private platformLocation: PlatformLocation, - private config: PlatformConfig) { - super(); - } - - handle(request: HttpRequest): Observable> { + override handle(request: HttpRequest): Observable> { const {href, protocol, hostname, port} = this.platformLocation; if (this.config.useAbsoluteUrl && !isAbsoluteUrl.test(request.url) && isAbsoluteUrl.test(href)) { @@ -118,27 +45,17 @@ export class ZoneClientBackend extends const urlPrefix = `${protocol}//${hostname}` + (port ? `:${port}` : ''); const baseUrl = new URL(baseHref, urlPrefix); const url = new URL(request.url, baseUrl); - return this.wrap(request.clone({url: url.toString()})); + return super.handle(request.clone({url: url.toString()})); } - return this.wrap(request); - } - protected override delegate(request: HttpRequest): Observable> { - return this.backend.handle(request); + return super.handle(request); } } -export function zoneWrappedInterceptorHandler( - platformLocation: PlatformLocation, config: PlatformConfig) { - return new ZoneClientBackend( - new HttpInterceptorHandler(inject(HttpBackend), inject(EnvironmentInjector)), - platformLocation, config); -} - export const SERVER_HTTP_PROVIDERS: Provider[] = [ {provide: XhrFactory, useClass: ServerXhr}, { provide: HttpHandler, - useFactory: zoneWrappedInterceptorHandler, - deps: [PlatformLocation, INITIAL_CONFIG] + useClass: ServerHttpInterceptorHandler, + deps: [PlatformLocation, INITIAL_CONFIG, HttpBackend, EnvironmentInjector] } ]; diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 9bc61af92a0..df49d5086ae 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -1339,23 +1339,6 @@ describe('platform-server integration', () => { }); }); - it('requests are macrotasks', waitForAsync(() => { - const platform = platformDynamicServer( - [{provide: INITIAL_CONFIG, useValue: {document: ''}}]); - platform.bootstrapModule(HttpClientExampleModule).then(ref => { - const mock = ref.injector.get(HttpTestingController) as HttpTestingController; - const http = ref.injector.get(HttpClient); - ref.injector.get(NgZone).run(() => { - http.get('http://localhost/testing').subscribe((body: string) => { - expect(body).toEqual('success!'); - }); - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy(); - mock.expectOne('http://localhost/testing').flush('success!'); - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeFalsy(); - }); - }); - })); - it('can use HttpInterceptor that injects HttpClient', () => { const platform = platformDynamicServer([{provide: INITIAL_CONFIG, useValue: {document: ''}}]);