fix(http): throw error if withFetch() and `withRequestsMadeViaParen…

[rainerhahnekamp]

These changes will ensure that an error is thrown when withRequestsMadeViaParent() and withFetch() are used together in the same call to provideHttpClient(), as this configuration is contradictory and would lead to problems in the execution, i.e. the interceptors from the parent's don't run.

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

If withFetch and withRequestsMadeViaParent() run together, the parents' interceptors don't work, and the user might not know why.

What is the new behavior?

It is the same as before, but with an additional error message during the dev mode.

Does this PR introduce a breaking change?

• Yes
• No

Other information

[AndrewKushnir]

@JeanMeche @rainerhahnekamp could you please provide a bit more info about the underlying issue? It'd be also great to capture this in the PR description and the commit message.

We should also mark this as a breaking change (include "BREAKING CHANGES" section into the commit message and PR description) and recommend developers to either remove withFetch or withRequestsMadeViaParent from app configurations.

[JeanMeche]

@AndrewKushnir here is a repro of the issue : https://stackblitz.com/edit/withrequestsmadeviaparent?file=src%2Fmain.ts&template=node

The FetchBackend cannot be overriden because of the PRIMARY_HTTP_BACKEND.
Which means that when if you declare another http provider with withRequestsMadeViaParent() and some new interceptors; Interceptors that were defined at the root at replaces.

This makes withFetch & withRequestsMadeViaParentcurrently incompatible.

[rainerhahnekamp]

To add a little bit of context: My application has global and feature-specific interceptors. Feature-specific are registered in lazy-loaded routes. That's why I must run provideHttpClient with withRequestsMadeViaParent.

withFetch has to be applied to every provideHttpClient. Otherwise, that instance would use xhr. As a result (explained by @JeanMeche above), the global interceptors weren't available for that particular HttpClient.

I see here three possibilities:

1. As already done in the PR, we throw an error if both features are used together. Unfortunately, that means I can't have any feature-specific interceptors AND fetch anymore.
2. We change the behavior so that the parent's interceptors are kept with withFetch. That would be a breaking change.
3. We add an option to withFetch like withFetch({applyToChildren: true}), which means fetch is also automatically used by all other child injectors. That would allow feature-specific interceptors and wouldn't introduce a breaking change. Additionally, we might have to introduce something like withXhr(). That would be an opt-out and would be used in child injectors.

[alan-agius4]

This seems like a bug and after I played with this a bit , I've also noticed an issue with a test case here, where HttpBackend should be HttpXhrBackend in that particular scenario since it is overridden explicitly.

I've made some adjustments to the code using the provided diff. Additionally, I've removed the private PRIMARY_HTTP_BACKEND while preserving the original concept behind it, where withFetch would be applied globally.

Here's the suggested modification which of course needs to be tested also in relation to angular-in-memory-web-api.

diff --git a/packages/common/http/src/interceptor.ts b/packages/common/http/src/interceptor.ts
index a77370e0f3..55c19183cd 100644
--- a/packages/common/http/src/interceptor.ts
+++ b/packages/common/http/src/interceptor.ts
@@ -280,12 +280,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.
diff --git a/packages/common/http/src/private_export.ts b/packages/common/http/src/private_export.ts
index cef681b02c..9855e86b1c 100644
--- a/packages/common/http/src/private_export.ts
+++ b/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';
diff --git a/packages/common/http/src/provider.ts b/packages/common/http/src/provider.ts
index 9905aaff99..23cac4d515 100644
--- a/packages/common/http/src/provider.ts
+++ b/packages/common/http/src/provider.ts
@@ -22,7 +22,6 @@ import {
   HttpInterceptorFn,
   HttpInterceptorHandler,
   legacyInterceptorFnFactory,
-  PRIMARY_HTTP_BACKEND,
 } from './interceptor';
 import {
   jsonpCallbackContext,
@@ -126,7 +125,13 @@ export function provideHttpClient(
     HttpXhrBackend,
     HttpInterceptorHandler,
     {provide: HttpHandler, useExisting: HttpInterceptorHandler},
-    {provide: HttpBackend, useExisting: HttpXhrBackend},
+    {
+      provide: HttpBackend,
+      useFactory: () =>
+        inject(FetchBackend, {
+          optional: true,
+        }) ?? inject(HttpXhrBackend),
+    },
     {
       provide: HTTP_INTERCEPTOR_FNS,
       useValue: xsrfInterceptorFn,
@@ -311,9 +316,5 @@ export function withFetch(): HttpFeature<HttpFeatureKind.Fetch> {
     );
   }
 
-  return makeHttpFeature(HttpFeatureKind.Fetch, [
-    FetchBackend,
-    {provide: HttpBackend, useExisting: FetchBackend},
-    {provide: PRIMARY_HTTP_BACKEND, useExisting: FetchBackend},
-  ]);
+  return makeHttpFeature(HttpFeatureKind.Fetch, [FetchBackend]);
 }

[alan-agius4]

Hi @rainerhahnekamp, thanks for your contribution. However, as mentioned earlier, this issue is a bug, so throwing an error isn't the appropriate solution.

I've prepared a pull request with the proper fix: #55652

[rainerhahnekamp]

@alan-agius4, thanks for the feedback. It is even better if the bug is fixed and no error is thrown 👍