From 501a6af3d655976c83dab43eb4762622a2fac524 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 3 May 2024 08:52:41 +0000 Subject: [PATCH] fix(http): resolve `withRequestsMadeViaParent` behavior with `withFetch` 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`. --- .../examples/ssr/src/app/app.config.ts | 3 +- packages/common/http/src/interceptor.ts | 13 -- packages/common/http/src/private_export.ts | 1 - packages/common/http/src/provider.ts | 21 +- packages/common/http/test/provider_spec.ts | 208 ++++++++++-------- .../http-client-in-memory-web-api-module.ts | 3 +- .../src/in-memory-web-api-module.ts | 3 +- 7 files changed, 127 insertions(+), 125 deletions(-) diff --git a/adev/src/content/examples/ssr/src/app/app.config.ts b/adev/src/content/examples/ssr/src/app/app.config.ts index c84cbed4227a..5b3c2018c21b 100644 --- a/adev/src/content/examples/ssr/src/app/app.config.ts +++ b/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 diff --git a/packages/common/http/src/interceptor.ts b/packages/common/http/src/interceptor.ts index a77370e0f343..5278e3d61262 100644 --- a/packages/common/http/src/interceptor.ts +++ b/packages/common/http/src/interceptor.ts @@ -215,13 +215,6 @@ export const HTTP_ROOT_INTERCEPTOR_FNS = new InjectionToken( - 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. diff --git a/packages/common/http/src/private_export.ts b/packages/common/http/src/private_export.ts index cef681b02c8e..9855e86b1c4b 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 9905aaff99a6..53128b8fd862 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,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 { - 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}, ]); } diff --git a/packages/common/http/test/provider_spec.ts b/packages/common/http/test/provider_spec.ts index 065d03ca75be..813cd68778a9 100644 --- a/packages/common/http/test/provider_spec.ts +++ b/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[] = []; + 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', () => { diff --git a/packages/misc/angular-in-memory-web-api/src/http-client-in-memory-web-api-module.ts b/packages/misc/angular-in-memory-web-api/src/http-client-in-memory-web-api-module.ts index 16c6686c99be..64473a34bdd9 100644 --- a/packages/misc/angular-in-memory-web-api/src/http-client-in-memory-web-api-module.ts +++ b/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}, ], }; } diff --git a/packages/misc/angular-in-memory-web-api/src/in-memory-web-api-module.ts b/packages/misc/angular-in-memory-web-api/src/in-memory-web-api-module.ts index 371d7d1d3893..6c106cae096b 100644 --- a/packages/misc/angular-in-memory-web-api/src/in-memory-web-api-module.ts +++ b/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}, ], }; }