fix(http): resolve withRequestsMadeViaParent behavior with withFetch (angular#55652)

This commit addresses dependency injection defects when using the `withFetch` API. Formerly, utilizing `withFetch` led to the automatic setting of `HttpBackend` to `FetchBackend`, which proved problematic in certain scenarios. Notably, conflicts arose when integrating `withRequestsMadeViaParent` and manually overriding tokens, as observed in instances like `InMemoryWebApiModule`.

PR Close angular#55652

Author: alan-agius4

adev/src/content/examples/ssr/src/app/app.config.ts

@@ -14,8 +14,7 @@ import {InMemoryDataService} from './in-memory-data.service';
 export const appConfig: ApplicationConfig = {
   providers: [
     provideRouter(routes),
-    // TODO: Enable using Fetch API when disabling `HttpClientInMemoryWebApiModule`.
-    provideHttpClient(/* withFetch()*/),
+    provideHttpClient(withFetch()),
     provideClientHydration(),
     provideProtractorTestingSupport(), // essential for e2e testing
 

packages/common/http/src/interceptor.ts

@@ -215,13 +215,6 @@ export const HTTP_ROOT_INTERCEPTOR_FNS = new InjectionToken<readonly HttpInterce
   ngDevMode ? 'HTTP_ROOT_INTERCEPTOR_FNS' : '',
 );
 
-/**
- * A provider to set a global primary http backend. If set, it will override the default one
- */
-export const PRIMARY_HTTP_BACKEND = new InjectionToken<HttpBackend>(
-  ngDevMode ? 'PRIMARY_HTTP_BACKEND' : '',
-);
-
 // TODO(atscott): We need a larger discussion about stability and what should contribute to stability.
 // Should the whole interceptor chain contribute to stability or just the backend request #55075?
 // Should HttpClient contribute to stability automatically at all?
@@ -280,12 +273,6 @@ export class HttpInterceptorHandler extends HttpHandler {
   ) {
     super();
 
-    // Check if there is a preferred HTTP backend configured and use it if that's the case.
-    // This is needed to enable `FetchBackend` globally for all HttpClient's when `withFetch`
-    // is used.
-    const primaryHttpBackend = inject(PRIMARY_HTTP_BACKEND, {optional: true});
-    this.backend = primaryHttpBackend ?? backend;
-
     // We strongly recommend using fetch backend for HTTP calls when SSR is used
     // for an application. The logic below checks if that's the case and produces
     // a warning otherwise.

packages/common/http/src/private_export.ts

@@ -8,6 +8,5 @@
 
 export {
   HTTP_ROOT_INTERCEPTOR_FNS as ɵHTTP_ROOT_INTERCEPTOR_FNS,
-  PRIMARY_HTTP_BACKEND as ɵPRIMARY_HTTP_BACKEND,
   REQUESTS_CONTRIBUTE_TO_STABILITY as ɵREQUESTS_CONTRIBUTE_TO_STABILITY,
 } from './interceptor';

packages/common/http/src/provider.ts

@@ -22,7 +22,6 @@ import {
   HttpInterceptorFn,
   HttpInterceptorHandler,
   legacyInterceptorFnFactory,
-  PRIMARY_HTTP_BACKEND,
 } from './interceptor';
 import {
   jsonpCallbackContext,
@@ -126,7 +125,12 @@ export function provideHttpClient(
     HttpXhrBackend,
     HttpInterceptorHandler,
     {provide: HttpHandler, useExisting: HttpInterceptorHandler},
-    {provide: HttpBackend, useExisting: HttpXhrBackend},
+    {
+      provide: HttpBackend,
+      useFactory: () => {
+        return inject(FetchBackend, {optional: true}) ?? inject(HttpXhrBackend);
+      },
+    },
     {
       provide: HTTP_INTERCEPTOR_FNS,
       useValue: xsrfInterceptorFn,
@@ -294,26 +298,13 @@ export function withRequestsMadeViaParent(): HttpFeature<HttpFeatureKind.Request
 /**
  * Configures the current `HttpClient` instance to make requests using the fetch API.
  *
- * This `FetchBackend` requires the support of the Fetch API which is available on all evergreen
- * browsers and on NodeJS from v18 onward.
- *
  * Note: The Fetch API doesn't support progress report on uploads.
  *
  * @publicApi
  */
 export function withFetch(): HttpFeature<HttpFeatureKind.Fetch> {
-  if ((typeof ngDevMode === 'undefined' || ngDevMode) && typeof fetch !== 'function') {
-    // TODO: Create a runtime error
-    // TODO: Use ENVIRONMENT_INITIALIZER to contextualize the error message (browser or server)
-    throw new Error(
-      'The `withFetch` feature of HttpClient requires the `fetch` API to be available. ' +
-        'If you run the code in a Node environment, make sure you use Node v18.10 or later.',
-    );
-  }
-
   return makeHttpFeature(HttpFeatureKind.Fetch, [
     FetchBackend,
     {provide: HttpBackend, useExisting: FetchBackend},
-    {provide: PRIMARY_HTTP_BACKEND, useExisting: FetchBackend},
   ]);
 }

packages/common/http/test/provider_spec.ts

@@ -40,6 +40,8 @@ import {EMPTY, Observable, from} from 'rxjs';
 
 import {HttpInterceptorFn, resetFetchBackendWarningFlag} from '../src/interceptor';
 import {
+  HttpFeature,
+  HttpFeatureKind,
   provideHttpClient,
   withFetch,
   withInterceptors,
@@ -339,94 +341,107 @@ describe('provideHttpClient', () => {
   });
 
   describe('withRequestsMadeViaParent()', () => {
-    it('should have independent HTTP setups if not explicitly specified', async () => {
-      TestBed.configureTestingModule({
-        providers: [provideHttpClient(), provideHttpClientTesting()],
-      });
-
-      const child = createEnvironmentInjector(
-        [
-          provideHttpClient(),
-          {
-            provide: XhrFactory,
-            useValue: {
-              build: () => {
-                throw new Error('Request reached the "backend".');
+    for (const backend of ['fetch', 'xhr']) {
+      describe(`given '${backend}' backend`, () => {
+        const commonHttpFeatures: HttpFeature<HttpFeatureKind>[] = [];
+        if (backend === 'fetch') {
+          commonHttpFeatures.push(withFetch());
+        }
+
+        it('should have independent HTTP setups if not explicitly specified', async () => {
+          TestBed.configureTestingModule({
+            providers: [provideHttpClient(...commonHttpFeatures), provideHttpClientTesting()],
+          });
+
+          const child = createEnvironmentInjector(
+            [
+              provideHttpClient(),
+              {
+                provide: XhrFactory,
+                useValue: {
+                  build: () => {
+                    throw new Error('Request reached the "backend".');
+                  },
+                },
               },
-            },
-          },
-        ],
-        TestBed.inject(EnvironmentInjector),
-      );
-
-      // Because `child` is an entirely independent HTTP context, it is not connected to the
-      // HTTP testing backend from the parent injector, and requests attempted via the child's
-      // `HttpClient` will fail.
-      await expectAsync(child.get(HttpClient).get('/test').toPromise()).toBeRejected();
-    });
-
-    it('should connect child to parent configuration if specified', () => {
-      TestBed.configureTestingModule({
-        providers: [provideHttpClient(), provideHttpClientTesting()],
-      });
-
-      const child = createEnvironmentInjector(
-        [provideHttpClient(withRequestsMadeViaParent())],
-        TestBed.inject(EnvironmentInjector),
-      );
-
-      // `child` is now to the parent HTTP context and therefore the testing backend, and so a
-      // request made via its `HttpClient` can be made.
-      child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
-      const req = TestBed.inject(HttpTestingController).expectOne('/test');
-      req.flush('');
-    });
-
-    it('should include interceptors from both parent and child contexts', () => {
-      TestBed.configureTestingModule({
-        providers: [
-          provideHttpClient(withInterceptors([makeLiteralTagInterceptorFn('parent')])),
-          provideHttpClientTesting(),
-        ],
-      });
+            ],
+            TestBed.inject(EnvironmentInjector),
+          );
+
+          // Because `child` is an entirely independent HTTP context, it is not connected to the
+          // HTTP testing backend from the parent injector, and requests attempted via the child's
+          // `HttpClient` will fail.
+          await expectAsync(child.get(HttpClient).get('/test').toPromise()).toBeRejected();
+        });
 
-      const child = createEnvironmentInjector(
-        [
-          provideHttpClient(
-            withRequestsMadeViaParent(),
-            withInterceptors([makeLiteralTagInterceptorFn('child')]),
-          ),
-        ],
-        TestBed.inject(EnvironmentInjector),
-      );
+        it('should connect child to parent configuration if specified', () => {
+          TestBed.configureTestingModule({
+            providers: [provideHttpClient(...commonHttpFeatures), provideHttpClientTesting()],
+          });
+
+          const child = createEnvironmentInjector(
+            [provideHttpClient(withRequestsMadeViaParent())],
+            TestBed.inject(EnvironmentInjector),
+          );
+
+          // `child` is now to the parent HTTP context and therefore the testing backend, and so a
+          // request made via its `HttpClient` can be made.
+          child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
+          const req = TestBed.inject(HttpTestingController).expectOne('/test');
+          req.flush('');
+        });
 
-      child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
-      const req = TestBed.inject(HttpTestingController).expectOne('/test');
-      expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
-      req.flush('');
-    });
+        it('should include interceptors from both parent and child contexts', () => {
+          TestBed.configureTestingModule({
+            providers: [
+              provideHttpClient(
+                ...commonHttpFeatures,
+                withInterceptors([makeLiteralTagInterceptorFn('parent')]),
+              ),
+              provideHttpClientTesting(),
+            ],
+          });
+
+          const child = createEnvironmentInjector(
+            [
+              provideHttpClient(
+                withRequestsMadeViaParent(),
+                withInterceptors([makeLiteralTagInterceptorFn('child')]),
+              ),
+            ],
+            TestBed.inject(EnvironmentInjector),
+          );
+
+          child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
+          const req = TestBed.inject(HttpTestingController).expectOne('/test');
+          expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
+          req.flush('');
+        });
 
-    it('should be able to connect to a legacy-provided HttpClient context', () => {
-      TestBed.configureTestingModule({
-        imports: [HttpClientTestingModule],
-        providers: [provideLegacyInterceptor('parent')],
+        it('should be able to connect to a legacy-provided HttpClient context', () => {
+          TestBed.configureTestingModule({
+            imports: [HttpClientTestingModule],
+            providers: [provideLegacyInterceptor('parent')],
+          });
+
+          const child = createEnvironmentInjector(
+            [
+              provideHttpClient(
+                ...commonHttpFeatures,
+                withRequestsMadeViaParent(),
+                withInterceptors([makeLiteralTagInterceptorFn('child')]),
+              ),
+            ],
+            TestBed.inject(EnvironmentInjector),
+          );
+
+          child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
+          const req = TestBed.inject(HttpTestingController).expectOne('/test');
+          expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
+          req.flush('');
+        });
       });
-
-      const child = createEnvironmentInjector(
-        [
-          provideHttpClient(
-            withRequestsMadeViaParent(),
-            withInterceptors([makeLiteralTagInterceptorFn('child')]),
-          ),
-        ],
-        TestBed.inject(EnvironmentInjector),
-      );
-
-      child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
-      const req = TestBed.inject(HttpTestingController).expectOne('/test');
-      expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
-      req.flush('');
-    });
+    }
   });
 
   describe('compatibility with Http NgModules', () => {
@@ -472,20 +487,33 @@ describe('provideHttpClient', () => {
       expect(consoleWarnSpy.calls.count()).toBe(0);
     });
 
-    it('withFetch should always override the backend', () => {
+    it(`'withFetch' should not override provided backend`, () => {
+      class CustomBackendExtends extends HttpXhrBackend {}
+
       TestBed.resetTestingModule();
       TestBed.configureTestingModule({
         providers: [
           provideHttpClient(withFetch()),
-          // This emulates a situation when `provideHttpClient()` is used
-          // later in a different part of an app. We want to make sure that
-          // the `FetchBackend` is enabled in that case as well.
-          {provide: HttpBackend, useClass: HttpXhrBackend},
+          {provide: HttpBackend, useClass: CustomBackendExtends},
         ],
       });
 
-      const handler = TestBed.inject(HttpHandler);
-      expect((handler as any).backend).toBeInstanceOf(FetchBackend);
+      const backend = TestBed.inject(HttpBackend);
+      expect(backend).toBeInstanceOf(CustomBackendExtends);
+    });
+
+    it(`fetch API should be used in child when 'withFetch' was used in parent injector`, () => {
+      TestBed.configureTestingModule({
+        providers: [provideHttpClient(withFetch()), provideHttpClientTesting()],
+      });
+
+      const child = createEnvironmentInjector(
+        [provideHttpClient()],
+        TestBed.inject(EnvironmentInjector),
+      );
+
+      const backend = child.get(HttpBackend);
+      expect(backend).toBeInstanceOf(FetchBackend);
     });
 
     it('should not warn if fetch is not configured when running in a browser', () => {

packages/misc/angular-in-memory-web-api/src/http-client-in-memory-web-api-module.ts

@@ -7,7 +7,7 @@
  */
 
 import {XhrFactory} from '@angular/common';
-import {HttpBackend, ɵPRIMARY_HTTP_BACKEND as PRIMARY_HTTP_BACKEND} from '@angular/common/http';
+import {HttpBackend} from '@angular/common/http';
 import {ModuleWithProviders, NgModule, Type} from '@angular/core';
 
 import {HttpClientBackendService} from './http-client-backend-service';
@@ -57,7 +57,6 @@ export class HttpClientInMemoryWebApiModule {
           useFactory: httpClientInMemBackendServiceFactory,
           deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
         },
-        {provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
       ],
     };
   }

packages/misc/angular-in-memory-web-api/src/in-memory-web-api-module.ts

@@ -7,7 +7,7 @@
  */
 
 import {XhrFactory} from '@angular/common';
-import {HttpBackend, ɵPRIMARY_HTTP_BACKEND as PRIMARY_HTTP_BACKEND} from '@angular/common/http';
+import {HttpBackend} from '@angular/common/http';
 import {ModuleWithProviders, NgModule, Type} from '@angular/core';
 
 import {httpClientInMemBackendServiceFactory} from './http-client-in-memory-web-api-module';
@@ -47,7 +47,6 @@ export class InMemoryWebApiModule {
           useFactory: httpClientInMemBackendServiceFactory,
           deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
         },
-        {provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
       ],
     };
   }