fix(http): force macro task creation during HTTP request (#49546)

This commit adds a background macrotask when an XHR request is performed. The macrotask is started during `loadstart` and ended during `loadend` event.

The macrotask is needed so that the application is not stabilized during HTTP calls. This is important for server rendering, as the application is rendering when the application is stabilized.
The application is stabilized when there are no longer pending Macro and Micro tasks intercepted by Zone.js, Since an XHR request is none of these, we create a background macrotask so that Zone.js is
made aware that there is something pending.

Prior to this change, we patched the `HttpHandler` in `@angular/platform-server` but this is not enough, as there can be multiple `HttpHandler` in an application, example when importing `HttpClient` in a lazy loaded component/module.
Which causes a new unpatched instance of `HttpHandler` to be created in the child injector which is not intercepted by Zone.js and thus the application is stabalized and rendered before the XHR request is finalized.

NB: Zone.js is fundamental for SSR and currently, it's not possible to do SSR without it.

Closes: #49425

PR Close #49546

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<T> {
 }
 
 // @public
-export class HttpXhrBackend implements HttpBackend {
+export class HttpXhrBackend implements HttpBackend, OnDestroy {
     constructor(xhrFactory: XhrFactory);
     handle(req: HttpRequest<any>): Observable<HttpEvent<any>>;
     // (undocumented)
+    ngOnDestroy(): void;
+    // (undocumented)
     static ɵfac: i0.ɵɵFactoryDeclaration<HttpXhrBackend, never>;
     // (undocumented)
     static ɵprov: i0.ɵɵInjectableDeclaration<HttpXhrBackend>;

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();
+  });
+});

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",

integration/platform-server/src/http-transferstate-lazy/app.component.ts

@@ -0,0 +1,8 @@
+import { Component } from '@angular/core';
+
+@Component({
+  selector: 'app-root',
+  template: '<router-outlet></router-outlet>',
+})
+export class AppComponent {
+}

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 {
+}

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 {
+}

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);
+};

integration/platform-server/src/http-transferstate-lazy/index.html

@@ -0,0 +1,11 @@
+<html>
+<head>
+  <base href="/http-transferstate-lazy">
+	<meta charset="UTF-8">
+	<title>Hello World</title>
+  <script src="webpack-out/httptransferstatelazy-bundle.js"></script>
+</head>
+<body>
+  <app-root></app-root>
+</body>
+</html>

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<string>('http');
+
+@Component({
+  selector: 'transfer-state-app-http',
+  template: `
+    <div>{{ response }}</div>
+  `,
+})
+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<any>(`http://localhost:4206/api`).subscribe((response) => {
+        this.transferState.set(httpCacheKey, response.data);
+        this.response = response.data;
+      });
+    } else {
+      this.response = this.transferState.get(httpCacheKey, '');
+    }
+  }
+}

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 {
+}

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<any>, html: string) {
@@ -25,7 +28,9 @@ function render(moduleType: Type<any>, 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!');
+});

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},

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);
 }

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<S, R> {
-  wrap(request: S): Observable<R> {
-    return new Observable((observer: Observer<R>) => {
-      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<R>;
-}
-
-export class ZoneClientBackend extends
-    ZoneMacroTaskWrapper<HttpRequest<any>, HttpEvent<any>> implements HttpBackend {
-  constructor(
-      private backend: HttpBackend, private platformLocation: PlatformLocation,
-      private config: PlatformConfig) {
-    super();
-  }
-
-  handle(request: HttpRequest<any>): Observable<HttpEvent<any>> {
+  override handle(request: HttpRequest<any>): Observable<HttpEvent<any>> {
     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<any>): Observable<HttpEvent<any>> {
-    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]
   }
 ];

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: '<app></app>'}}]);
-         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<string>('http://localhost/testing').subscribe((body: string) => {
-               expect(body).toEqual('success!');
-             });
-             expect(ref.injector.get<NgZone>(NgZone).hasPendingMacrotasks).toBeTruthy();
-             mock.expectOne('http://localhost/testing').flush('success!');
-             expect(ref.injector.get<NgZone>(NgZone).hasPendingMacrotasks).toBeFalsy();
-           });
-         });
-       }));
-
     it('can use HttpInterceptor that injects HttpClient', () => {
       const platform =
           platformDynamicServer([{provide: INITIAL_CONFIG, useValue: {document: '<app></app>'}}]);