Skip to content

Commit 501a6af

Browse files
alan-agius4alan-agius4
committed
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`.
1 parent 58a8f12 commit 501a6af

7 files changed

Lines changed: 127 additions & 125 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ import {InMemoryDataService} from './in-memory-data.service';
1414
export const appConfig: ApplicationConfig = {
1515
providers: [
1616
provideRouter(routes),
17-
// TODO: Enable using Fetch API when disabling `HttpClientInMemoryWebApiModule`.
18-
provideHttpClient(/* withFetch()*/),
17+
provideHttpClient(withFetch()),
1918
provideClientHydration(),
2019
provideProtractorTestingSupport(), // essential for e2e testing
2120

‎packages/common/http/src/interceptor.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,6 @@ export const HTTP_ROOT_INTERCEPTOR_FNS = new InjectionToken<readonly HttpInterce
215215
ngDevMode ? 'HTTP_ROOT_INTERCEPTOR_FNS' : '',
216216
);
217217

218-
/**
219-
* A provider to set a global primary http backend. If set, it will override the default one
220-
*/
221-
export const PRIMARY_HTTP_BACKEND = new InjectionToken<HttpBackend>(
222-
ngDevMode ? 'PRIMARY_HTTP_BACKEND' : '',
223-
);
224-
225218
// TODO(atscott): We need a larger discussion about stability and what should contribute to stability.
226219
// Should the whole interceptor chain contribute to stability or just the backend request #55075?
227220
// Should HttpClient contribute to stability automatically at all?
@@ -280,12 +273,6 @@ export class HttpInterceptorHandler extends HttpHandler {
280273
) {
281274
super();
282275

283-
// Check if there is a preferred HTTP backend configured and use it if that's the case.
284-
// This is needed to enable `FetchBackend` globally for all HttpClient's when `withFetch`
285-
// is used.
286-
const primaryHttpBackend = inject(PRIMARY_HTTP_BACKEND, {optional: true});
287-
this.backend = primaryHttpBackend ?? backend;
288-
289276
// We strongly recommend using fetch backend for HTTP calls when SSR is used
290277
// for an application. The logic below checks if that's the case and produces
291278
// a warning otherwise.

‎packages/common/http/src/private_export.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,5 @@
88

99
export {
1010
HTTP_ROOT_INTERCEPTOR_FNS as ɵHTTP_ROOT_INTERCEPTOR_FNS,
11-
PRIMARY_HTTP_BACKEND as ɵPRIMARY_HTTP_BACKEND,
1211
REQUESTS_CONTRIBUTE_TO_STABILITY as ɵREQUESTS_CONTRIBUTE_TO_STABILITY,
1312
} from './interceptor';

‎packages/common/http/src/provider.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
HttpInterceptorFn,
2323
HttpInterceptorHandler,
2424
legacyInterceptorFnFactory,
25-
PRIMARY_HTTP_BACKEND,
2625
} from './interceptor';
2726
import {
2827
jsonpCallbackContext,
@@ -126,7 +125,12 @@ export function provideHttpClient(
126125
HttpXhrBackend,
127126
HttpInterceptorHandler,
128127
{provide: HttpHandler, useExisting: HttpInterceptorHandler},
129-
{provide: HttpBackend, useExisting: HttpXhrBackend},
128+
{
129+
provide: HttpBackend,
130+
useFactory: () => {
131+
return inject(FetchBackend, {optional: true}) ?? inject(HttpXhrBackend);
132+
},
133+
},
130134
{
131135
provide: HTTP_INTERCEPTOR_FNS,
132136
useValue: xsrfInterceptorFn,
@@ -294,26 +298,13 @@ export function withRequestsMadeViaParent(): HttpFeature<HttpFeatureKind.Request
294298
/**
295299
* Configures the current `HttpClient` instance to make requests using the fetch API.
296300
*
297-
* This `FetchBackend` requires the support of the Fetch API which is available on all evergreen
298-
* browsers and on NodeJS from v18 onward.
299-
*
300301
* Note: The Fetch API doesn't support progress report on uploads.
301302
*
302303
* @publicApi
303304
*/
304305
export function withFetch(): HttpFeature<HttpFeatureKind.Fetch> {
305-
if ((typeof ngDevMode === 'undefined' || ngDevMode) && typeof fetch !== 'function') {
306-
// TODO: Create a runtime error
307-
// TODO: Use ENVIRONMENT_INITIALIZER to contextualize the error message (browser or server)
308-
throw new Error(
309-
'The `withFetch` feature of HttpClient requires the `fetch` API to be available. ' +
310-
'If you run the code in a Node environment, make sure you use Node v18.10 or later.',
311-
);
312-
}
313-
314306
return makeHttpFeature(HttpFeatureKind.Fetch, [
315307
FetchBackend,
316308
{provide: HttpBackend, useExisting: FetchBackend},
317-
{provide: PRIMARY_HTTP_BACKEND, useExisting: FetchBackend},
318309
]);
319310
}

‎packages/common/http/test/provider_spec.ts‎

Lines changed: 118 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import {EMPTY, Observable, from} from 'rxjs';
4040

4141
import {HttpInterceptorFn, resetFetchBackendWarningFlag} from '../src/interceptor';
4242
import {
43+
HttpFeature,
44+
HttpFeatureKind,
4345
provideHttpClient,
4446
withFetch,
4547
withInterceptors,
@@ -339,94 +341,107 @@ describe('provideHttpClient', () => {
339341
});
340342

341343
describe('withRequestsMadeViaParent()', () => {
342-
it('should have independent HTTP setups if not explicitly specified', async () => {
343-
TestBed.configureTestingModule({
344-
providers: [provideHttpClient(), provideHttpClientTesting()],
345-
});
346-
347-
const child = createEnvironmentInjector(
348-
[
349-
provideHttpClient(),
350-
{
351-
provide: XhrFactory,
352-
useValue: {
353-
build: () => {
354-
throw new Error('Request reached the "backend".');
344+
for (const backend of ['fetch', 'xhr']) {
345+
describe(`given '${backend}' backend`, () => {
346+
const commonHttpFeatures: HttpFeature<HttpFeatureKind>[] = [];
347+
if (backend === 'fetch') {
348+
commonHttpFeatures.push(withFetch());
349+
}
350+
351+
it('should have independent HTTP setups if not explicitly specified', async () => {
352+
TestBed.configureTestingModule({
353+
providers: [provideHttpClient(...commonHttpFeatures), provideHttpClientTesting()],
354+
});
355+
356+
const child = createEnvironmentInjector(
357+
[
358+
provideHttpClient(),
359+
{
360+
provide: XhrFactory,
361+
useValue: {
362+
build: () => {
363+
throw new Error('Request reached the "backend".');
364+
},
365+
},
355366
},
356-
},
357-
},
358-
],
359-
TestBed.inject(EnvironmentInjector),
360-
);
361-
362-
// Because `child` is an entirely independent HTTP context, it is not connected to the
363-
// HTTP testing backend from the parent injector, and requests attempted via the child's
364-
// `HttpClient` will fail.
365-
await expectAsync(child.get(HttpClient).get('/test').toPromise()).toBeRejected();
366-
});
367-
368-
it('should connect child to parent configuration if specified', () => {
369-
TestBed.configureTestingModule({
370-
providers: [provideHttpClient(), provideHttpClientTesting()],
371-
});
372-
373-
const child = createEnvironmentInjector(
374-
[provideHttpClient(withRequestsMadeViaParent())],
375-
TestBed.inject(EnvironmentInjector),
376-
);
377-
378-
// `child` is now to the parent HTTP context and therefore the testing backend, and so a
379-
// request made via its `HttpClient` can be made.
380-
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
381-
const req = TestBed.inject(HttpTestingController).expectOne('/test');
382-
req.flush('');
383-
});
384-
385-
it('should include interceptors from both parent and child contexts', () => {
386-
TestBed.configureTestingModule({
387-
providers: [
388-
provideHttpClient(withInterceptors([makeLiteralTagInterceptorFn('parent')])),
389-
provideHttpClientTesting(),
390-
],
391-
});
367+
],
368+
TestBed.inject(EnvironmentInjector),
369+
);
370+
371+
// Because `child` is an entirely independent HTTP context, it is not connected to the
372+
// HTTP testing backend from the parent injector, and requests attempted via the child's
373+
// `HttpClient` will fail.
374+
await expectAsync(child.get(HttpClient).get('/test').toPromise()).toBeRejected();
375+
});
392376

393-
const child = createEnvironmentInjector(
394-
[
395-
provideHttpClient(
396-
withRequestsMadeViaParent(),
397-
withInterceptors([makeLiteralTagInterceptorFn('child')]),
398-
),
399-
],
400-
TestBed.inject(EnvironmentInjector),
401-
);
377+
it('should connect child to parent configuration if specified', () => {
378+
TestBed.configureTestingModule({
379+
providers: [provideHttpClient(...commonHttpFeatures), provideHttpClientTesting()],
380+
});
381+
382+
const child = createEnvironmentInjector(
383+
[provideHttpClient(withRequestsMadeViaParent())],
384+
TestBed.inject(EnvironmentInjector),
385+
);
386+
387+
// `child` is now to the parent HTTP context and therefore the testing backend, and so a
388+
// request made via its `HttpClient` can be made.
389+
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
390+
const req = TestBed.inject(HttpTestingController).expectOne('/test');
391+
req.flush('');
392+
});
402393

403-
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
404-
const req = TestBed.inject(HttpTestingController).expectOne('/test');
405-
expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
406-
req.flush('');
407-
});
394+
it('should include interceptors from both parent and child contexts', () => {
395+
TestBed.configureTestingModule({
396+
providers: [
397+
provideHttpClient(
398+
...commonHttpFeatures,
399+
withInterceptors([makeLiteralTagInterceptorFn('parent')]),
400+
),
401+
provideHttpClientTesting(),
402+
],
403+
});
404+
405+
const child = createEnvironmentInjector(
406+
[
407+
provideHttpClient(
408+
withRequestsMadeViaParent(),
409+
withInterceptors([makeLiteralTagInterceptorFn('child')]),
410+
),
411+
],
412+
TestBed.inject(EnvironmentInjector),
413+
);
414+
415+
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
416+
const req = TestBed.inject(HttpTestingController).expectOne('/test');
417+
expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
418+
req.flush('');
419+
});
408420

409-
it('should be able to connect to a legacy-provided HttpClient context', () => {
410-
TestBed.configureTestingModule({
411-
imports: [HttpClientTestingModule],
412-
providers: [provideLegacyInterceptor('parent')],
421+
it('should be able to connect to a legacy-provided HttpClient context', () => {
422+
TestBed.configureTestingModule({
423+
imports: [HttpClientTestingModule],
424+
providers: [provideLegacyInterceptor('parent')],
425+
});
426+
427+
const child = createEnvironmentInjector(
428+
[
429+
provideHttpClient(
430+
...commonHttpFeatures,
431+
withRequestsMadeViaParent(),
432+
withInterceptors([makeLiteralTagInterceptorFn('child')]),
433+
),
434+
],
435+
TestBed.inject(EnvironmentInjector),
436+
);
437+
438+
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
439+
const req = TestBed.inject(HttpTestingController).expectOne('/test');
440+
expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
441+
req.flush('');
442+
});
413443
});
414-
415-
const child = createEnvironmentInjector(
416-
[
417-
provideHttpClient(
418-
withRequestsMadeViaParent(),
419-
withInterceptors([makeLiteralTagInterceptorFn('child')]),
420-
),
421-
],
422-
TestBed.inject(EnvironmentInjector),
423-
);
424-
425-
child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe();
426-
const req = TestBed.inject(HttpTestingController).expectOne('/test');
427-
expect(req.request.headers.get('X-Tag')).toEqual('child,parent');
428-
req.flush('');
429-
});
444+
}
430445
});
431446

432447
describe('compatibility with Http NgModules', () => {
@@ -472,20 +487,33 @@ describe('provideHttpClient', () => {
472487
expect(consoleWarnSpy.calls.count()).toBe(0);
473488
});
474489

475-
it('withFetch should always override the backend', () => {
490+
it(`'withFetch' should not override provided backend`, () => {
491+
class CustomBackendExtends extends HttpXhrBackend {}
492+
476493
TestBed.resetTestingModule();
477494
TestBed.configureTestingModule({
478495
providers: [
479496
provideHttpClient(withFetch()),
480-
// This emulates a situation when `provideHttpClient()` is used
481-
// later in a different part of an app. We want to make sure that
482-
// the `FetchBackend` is enabled in that case as well.
483-
{provide: HttpBackend, useClass: HttpXhrBackend},
497+
{provide: HttpBackend, useClass: CustomBackendExtends},
484498
],
485499
});
486500

487-
const handler = TestBed.inject(HttpHandler);
488-
expect((handler as any).backend).toBeInstanceOf(FetchBackend);
501+
const backend = TestBed.inject(HttpBackend);
502+
expect(backend).toBeInstanceOf(CustomBackendExtends);
503+
});
504+
505+
it(`fetch API should be used in child when 'withFetch' was used in parent injector`, () => {
506+
TestBed.configureTestingModule({
507+
providers: [provideHttpClient(withFetch()), provideHttpClientTesting()],
508+
});
509+
510+
const child = createEnvironmentInjector(
511+
[provideHttpClient()],
512+
TestBed.inject(EnvironmentInjector),
513+
);
514+
515+
const backend = child.get(HttpBackend);
516+
expect(backend).toBeInstanceOf(FetchBackend);
489517
});
490518

491519
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‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {XhrFactory} from '@angular/common';
10-
import {HttpBackend, ɵPRIMARY_HTTP_BACKEND as PRIMARY_HTTP_BACKEND} from '@angular/common/http';
10+
import {HttpBackend} from '@angular/common/http';
1111
import {ModuleWithProviders, NgModule, Type} from '@angular/core';
1212

1313
import {HttpClientBackendService} from './http-client-backend-service';
@@ -57,7 +57,6 @@ export class HttpClientInMemoryWebApiModule {
5757
useFactory: httpClientInMemBackendServiceFactory,
5858
deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
5959
},
60-
{provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
6160
],
6261
};
6362
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {XhrFactory} from '@angular/common';
10-
import {HttpBackend, ɵPRIMARY_HTTP_BACKEND as PRIMARY_HTTP_BACKEND} from '@angular/common/http';
10+
import {HttpBackend} from '@angular/common/http';
1111
import {ModuleWithProviders, NgModule, Type} from '@angular/core';
1212

1313
import {httpClientInMemBackendServiceFactory} from './http-client-in-memory-web-api-module';
@@ -47,7 +47,6 @@ export class InMemoryWebApiModule {
4747
useFactory: httpClientInMemBackendServiceFactory,
4848
deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
4949
},
50-
{provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
5150
],
5251
};
5352
}

0 commit comments

Comments
 (0)