Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alan-agius4 wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions adev/src/content/examples/ssr/src/app/app.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 0 additions & 13 deletions packages/common/http/src/interceptor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion packages/common/http/src/private_export.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
21 changes: 6 additions & 15 deletions packages/common/http/src/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
HttpInterceptorFn,
HttpInterceptorHandler,
legacyInterceptorFnFactory,
PRIMARY_HTTP_BACKEND,
} from './interceptor';
import {
jsonpCallbackContext,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

@alan-agius4 alan-agius4 May 3, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are no longer needed as with v18 all versions of Node.js support fetch

* 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},
]);
}
208 changes: 118 additions & 90 deletions packages/common/http/test/provider_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import {EMPTY, Observable, from} from 'rxjs';

import {HttpInterceptorFn, resetFetchBackendWarningFlag} from '../src/interceptor';
import {
HttpFeature,
HttpFeatureKind,
provideHttpClient,
withFetch,
withInterceptors,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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`, () => {
Comment thread
JeanMeche marked this conversation as resolved.
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', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/misc/angular-in-memory-web-api/src/http-client-in-memory-web-api-module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -57,7 +57,6 @@ export class HttpClientInMemoryWebApiModule {
useFactory: httpClientInMemBackendServiceFactory,
deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
},
{provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
],
};
}
Expand Down
3 changes: 1 addition & 2 deletions packages/misc/angular-in-memory-web-api/src/in-memory-web-api-module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -47,7 +47,6 @@ export class InMemoryWebApiModule {
useFactory: httpClientInMemBackendServiceFactory,
deps: [InMemoryDbService, InMemoryBackendConfig, XhrFactory],
},
{provide: PRIMARY_HTTP_BACKEND, useExisting: HttpBackend},
],
};
}
Expand Down