Skip to content

Commit

Permalink
refactor(http): replace zone.js macrotask creation with `InitialRende…
Browse files Browse the repository at this point in the history
…rPendingTasks`

This commits refactors the HTTP client to use `InitialRenderPendingTasks` insteas of Zone.js macrotask. This is another approach to angular#50406 which was revert due to a failure in G3.
  • Loading branch information
alan-agius4 committed May 26, 2023
1 parent 3152d06 commit 2057930
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 45 deletions.
13 changes: 10 additions & 3 deletions packages/common/http/src/interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {EnvironmentInjector, inject, Injectable, InjectionToken} from '@angular/core';
import {EnvironmentInjector, inject, Injectable, InjectionToken, ɵInitialRenderPendingTasks as InitialRenderPendingTasks} from '@angular/core';
import {Observable} from 'rxjs';
import {finalize} from 'rxjs/operators';

import {HttpBackend, HttpHandler} from './backend';
import {HttpRequest} from './request';
Expand Down Expand Up @@ -178,13 +179,16 @@ export function legacyInterceptorFnFactory(): HttpInterceptorFn {
adaptLegacyInterceptorToChain, interceptorChainEndFn as ChainedInterceptorFn<any>);
}

return chain(req, handler);
const pendingTasks = inject(InitialRenderPendingTasks);
const taskId = pendingTasks.add();
return chain(req, handler).pipe(finalize(() => pendingTasks.remove(taskId)));
};
}

@Injectable()
export class HttpInterceptorHandler extends HttpHandler {
private chain: ChainedInterceptorFn<unknown>|null = null;
private readonly pendingTasks = inject(InitialRenderPendingTasks);

constructor(private backend: HttpBackend, private injector: EnvironmentInjector) {
super();
Expand All @@ -206,6 +210,9 @@ export class HttpInterceptorHandler extends HttpHandler {
chainedInterceptorFn(nextSequencedFn, interceptorFn, this.injector),
interceptorChainEndFn as ChainedInterceptorFn<unknown>);
}
return this.chain(initialRequest, downstreamRequest => this.backend.handle(downstreamRequest));

const taskId = this.pendingTasks.add();
return this.chain(initialRequest, downstreamRequest => this.backend.handle(downstreamRequest))
.pipe(finalize(() => this.pendingTasks.remove(taskId)));
}
}
36 changes: 0 additions & 36 deletions packages/common/http/src/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,36 +309,18 @@ export class HttpXhrBackend implements HttpBackend {
}
}

let macroTaskCanceller: VoidFunction|undefined;

/** Tear down logic to cancel the backround macrotask. */
const onLoadStart = () => {
macroTaskCanceller ??= createBackgroundMacroTask();
};
const onLoadEnd = () => {
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.
macroTaskCanceller?.();

if (req.reportProgress) {
xhr.removeEventListener('progress', onDownProgress);
if (reqBody !== null && xhr.upload) {
Expand All @@ -356,21 +338,3 @@ export class HttpXhrBackend implements HttpBackend {
);
}
}

// 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);
}
18 changes: 13 additions & 5 deletions packages/core/src/initial_render_pending_tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Injectable} from './di';
import {inject, Injectable} from './di';
import {OnDestroy} from './interface/lifecycle_hooks';
import {NgZone} from './zone';

/**
* *Internal* service that keeps track of pending tasks happening in the system
Expand All @@ -23,6 +24,7 @@ export class InitialRenderPendingTasks implements OnDestroy {
private taskId = 0;
private pendingTasks = new Set<number>();
private completed = false;
private ngZone = inject(NgZone);

get hasPendingTasks(): boolean {
return this.pendingTasks.size > 0;
Expand All @@ -42,10 +44,16 @@ export class InitialRenderPendingTasks implements OnDestroy {
remove(taskId: number): void {
if (this.completed) return;

this.pendingTasks.delete(taskId);
if (this.pendingTasks.size === 0) {
this.complete();
}
this.ngZone.runOutsideAngular(() => {
// Run outside of the Angular zone to avoid triggering
// extra change detection cycles.
setTimeout(() => {
this.pendingTasks.delete(taskId);
if (this.pendingTasks.size === 0) {
this.complete();
}
}, 0);
});
}

ngOnDestroy(): void {
Expand Down
2 changes: 1 addition & 1 deletion packages/router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ export class Router {
afterNextNavigation(this, () => {
// Remove pending task in a microtask to allow for cancelled
// initial navigations and redirects within the same task.
Promise.resolve().then(() => this.pendingTasks.remove(taskId));
this.pendingTasks.remove(taskId);
});

this.navigationTransitions.handleNavigationRequest({
Expand Down

0 comments on commit 2057930

Please sign in to comment.