Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions goldens/public-api/common/http/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,9 @@ export enum HttpFeatureKind {
// (undocumented)
NoXsrfProtection = 3,
// (undocumented)
RequestsMadeViaParent = 5
RequestsMadeViaParent = 5,
// (undocumented)
Xhr = 7
}

// @public
Expand Down Expand Up @@ -2001,7 +2003,7 @@ export class JsonpInterceptor {
// @public
export function provideHttpClient(...features: HttpFeature<HttpFeatureKind>[]): EnvironmentProviders;

// @public
// @public @deprecated
export function withFetch(): HttpFeature<HttpFeatureKind.Fetch>;

// @public
Expand All @@ -2019,6 +2021,9 @@ export function withNoXsrfProtection(): HttpFeature<HttpFeatureKind.NoXsrfProtec
// @public
export function withRequestsMadeViaParent(): HttpFeature<HttpFeatureKind.RequestsMadeViaParent>;

// @public
export function withXhr(): HttpFeature<HttpFeatureKind.Xhr>;

// @public
export function withXsrfConfiguration({ cookieName, headerName, }: {
cookieName?: string;
Expand Down
1 change: 1 addition & 0 deletions packages/common/http/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export {
HttpFeatureKind,
provideHttpClient,
withFetch,
withXhr,
withInterceptors,
withInterceptorsFromDi,
withJsonpSupport,
Expand Down
15 changes: 15 additions & 0 deletions packages/common/http/src/backend-default-value.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {FetchBackend} from './fetch';

/**
* A constant defining the default the default Http Backend.
* Extracted to a separate file to facilitate G3 patches.
*/
export const NG_DEFAULT_HTTP_BACKEND = FetchBackend;
24 changes: 21 additions & 3 deletions packages/common/http/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
XSRF_HEADER_NAME,
xsrfInterceptorFn,
} from './xsrf';
import {NG_DEFAULT_HTTP_BACKEND} from './backend-default-value';

/**
* Identifies a particular kind of `HttpFeature`.
Expand All @@ -52,6 +53,7 @@ export enum HttpFeatureKind {
JsonpSupport,
RequestsMadeViaParent,
Fetch,
Xhr,
}

/**
Expand Down Expand Up @@ -101,7 +103,7 @@ function makeHttpFeature<KindT extends HttpFeatureKind>(
* @see {@link withNoXsrfProtection}
* @see {@link withJsonpSupport}
* @see {@link withRequestsMadeViaParent}
* @see {@link withFetch}
* @see {@link withXhr}
*/
export function provideHttpClient(
...features: HttpFeature<HttpFeatureKind>[]
Expand All @@ -122,13 +124,13 @@ export function provideHttpClient(

const providers: Provider[] = [
HttpClient,
HttpXhrBackend,
NG_DEFAULT_HTTP_BACKEND,
HttpInterceptorHandler,
{provide: HttpHandler, useExisting: HttpInterceptorHandler},
{
provide: HttpBackend,
useFactory: () => {
return inject(FETCH_BACKEND, {optional: true}) ?? inject(HttpXhrBackend);
return inject(FETCH_BACKEND, {optional: true}) ?? inject(NG_DEFAULT_HTTP_BACKEND);
},
},
{
Expand Down Expand Up @@ -301,6 +303,7 @@ export function withRequestsMadeViaParent(): HttpFeature<HttpFeatureKind.Request
* Note: The Fetch API doesn't support progress report on uploads.
*
* @publicApi
* @deprecated `withFetch` is not required anymore. `FetchBackend` is the default `HttpBackend`.
*/
export function withFetch(): HttpFeature<HttpFeatureKind.Fetch> {
return makeHttpFeature(HttpFeatureKind.Fetch, [
Expand All @@ -309,3 +312,18 @@ export function withFetch(): HttpFeature<HttpFeatureKind.Fetch> {
{provide: HttpBackend, useExisting: FetchBackend},
]);
}

/**
* Configures the current `HttpClient` instance to make requests using the Xhr API.
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be meaningful to extend this a bit with information on what this offers, especially its support for upload progress events.

Thinking about it more, do we already document this limitation of fetch for the 'progress' response mode? We should probably make it clear there as well that withXhr is needed to make it work. Does the fetch backend report an error somehow if progress is requested? Is this actually only about upload progress, or also download progress?

Copy link
Member Author

Choose a reason for hiding this comment

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

Chrome is the only browser that supports a stream as a body (since M105), every browser support streaming the response (hence the support for download progress report).

The FetchBackend doesn't throw any error, it won't just report any progress on uploads. Currently the API only has a single reportProgress property for both update & download progress report, I don't think we should log any warning.

I've updated the doc for the reportProgress property to hint a the new limitation with the default implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

For this use case, it's unfortunate that reportProgress is used for both upload and download, which is why I understand that you don't want a warning. However, given it doesn't seem to be possible to use duplex: "half" with Angular's fetch-backend yet (correct me if I'm wrong), I think a warning would still be good if you set reportProgress to true in dev mode, regardless - at least for now.

Not sure what the ratio is of devs using it to report uploads vs. downloads, but from my anecdotal experience, it's usually for uploads.

*
* Use this feature if you want to report progress on uploads as the Xhr API supports it.
*
* @see {@link provideHttpClient}
* @publicApi
*/
export function withXhr(): HttpFeature<HttpFeatureKind.Xhr> {
return makeHttpFeature(HttpFeatureKind.Xhr, [
HttpXhrBackend,
{provide: HttpBackend, useExisting: HttpXhrBackend},
]);
}
3 changes: 2 additions & 1 deletion packages/common/http/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ export class HttpRequest<T> {
* Progress events are expensive (change detection runs on each event) and so
* they should only be requested if the consumer intends to monitor them.
*
* Note: The `FetchBackend` doesn't support progress report on uploads.
* Note: The default `HttpBackend` based on fetch, does not support progress report for uploads.
* Set the `HttpXhrBackend` with `withXhr()` if you need this feature.
*/
readonly reportProgress: boolean = false;

Expand Down
3 changes: 1 addition & 2 deletions packages/common/http/test/fetch_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
HttpParams,
HttpStatusCode,
provideHttpClient,
withFetch,
} from '../public_api';
import {FetchBackend, FetchFactory} from '../src/fetch';

Expand Down Expand Up @@ -424,7 +423,7 @@ describe('FetchBackend', async () => {
beforeEach(() => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
providers: [provideHttpClient(withFetch())],
providers: [provideHttpClient()],
});
});

Expand Down
17 changes: 7 additions & 10 deletions packages/common/http/test/provider_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ import {
HttpFeature,
HttpFeatureKind,
provideHttpClient,
withFetch,
withInterceptors,
withInterceptorsFromDi,
withJsonpSupport,
withNoXsrfProtection,
withRequestsMadeViaParent,
withXhr,
withXsrfConfiguration,
} from '../src/provider';

Expand Down Expand Up @@ -340,8 +340,8 @@ describe('provideHttpClient', () => {
for (const backend of ['fetch', 'xhr']) {
describe(`given '${backend}' backend`, () => {
const commonHttpFeatures: HttpFeature<HttpFeatureKind>[] = [];
if (backend === 'fetch') {
commonHttpFeatures.push(withFetch());
if (backend === 'xhr') {
commonHttpFeatures.push(withXhr());
}

it('should have independent HTTP setups if not explicitly specified', async () => {
Expand Down Expand Up @@ -473,7 +473,7 @@ describe('provideHttpClient', () => {
// `console.warn` produced for cases when `fetch`
// is enabled and we are running in a browser.
{provide: PLATFORM_ID, useValue: 'browser'},
provideHttpClient(withFetch()),
provideHttpClient(),
],
});
const fetchBackend = TestBed.inject(HttpBackend);
Expand All @@ -488,10 +488,7 @@ describe('provideHttpClient', () => {

TestBed.resetTestingModule();
TestBed.configureTestingModule({
providers: [
provideHttpClient(withFetch()),
{provide: HttpBackend, useClass: CustomBackendExtends},
],
providers: [provideHttpClient(), {provide: HttpBackend, useClass: CustomBackendExtends}],
});

const backend = TestBed.inject(HttpBackend);
Expand All @@ -500,7 +497,7 @@ describe('provideHttpClient', () => {

it(`fetch API should be used in child when 'withFetch' was used in parent injector`, () => {
TestBed.configureTestingModule({
providers: [provideHttpClient(withFetch()), provideHttpClientTesting()],
providers: [provideHttpClient(), provideHttpClientTesting()],
});

const child = createEnvironmentInjector(
Expand Down Expand Up @@ -546,7 +543,7 @@ describe('provideHttpClient', () => {
// `console.warn` produced in case `fetch` is not
// enabled while running code on the server.
{provide: PLATFORM_ID, useValue: 'server'},
provideHttpClient(),
provideHttpClient(withXhr()),
],
});

Expand Down
2 changes: 2 additions & 0 deletions packages/core/schematics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ rollup_bundle(
"//packages/core/schematics/migrations/test-bed-get:index.ts": "test-bed-get",
"//packages/core/schematics/migrations/document-core:index.ts": "document-core",
"//packages/core/schematics/migrations/control-flow-migration:index.ts": "control-flow-migration",
"//packages/core/schematics/migrations/http-xhr-backend:index.ts": "http-xhr-backend",
},
format = "cjs",
link_workspace_root = True,
Expand All @@ -58,6 +59,7 @@ rollup_bundle(
deps = [
"//packages/core/schematics/migrations/control-flow-migration",
"//packages/core/schematics/migrations/document-core",
"//packages/core/schematics/migrations/http-xhr-backend",
"//packages/core/schematics/migrations/inject-flags",
"//packages/core/schematics/migrations/test-bed-get",
"//packages/core/schematics/ng-generate/cleanup-unused-imports",
Expand Down
5 changes: 5 additions & 0 deletions packages/core/schematics/migrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
"version": "20.0.0",
"description": "Moves imports of `DOCUMENT` from `@angular/common` to `@angular/core`",
"factory": "./bundles/document-core#migrate"
},
"http-xhr-backend": {
"version": "19.0.0",
"description": "Adds 'withXhr' to 'provideHttpClient' function calls when the 'HttpXhrBackend' is used. For more information see: https://angular.dev/api/common/http/withXhr",
"factory": "./bundles/http-xhr-backend#migrate"
}
}
}
21 changes: 21 additions & 0 deletions packages/core/schematics/migrations/http-xhr-backend/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
load("//tools:defaults.bzl", "ts_library")

package(
default_visibility = [
"//packages/core/schematics:__pkg__",
"//packages/core/schematics/migrations/google3:__pkg__",
"//packages/core/schematics/test:__pkg__",
],
)

ts_library(
name = "http-xhr-backend",
srcs = glob(["**/*.ts"]),
tsconfig = "//packages/core/schematics:tsconfig.json",
deps = [
"//packages/core/schematics/utils",
"@npm//@angular-devkit/schematics",
"@npm//@types/node",
"@npm//typescript",
],
)
58 changes: 58 additions & 0 deletions packages/core/schematics/migrations/http-xhr-backend/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {Rule, SchematicsException, Tree, UpdateRecorder} from '@angular-devkit/schematics';
import {relative} from 'path';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {migrateFile} from './migration';

export function migrate(): Rule {
return async (tree: Tree) => {
const {buildPaths, testPaths} = await getProjectTsConfigPaths(tree);
const basePath = process.cwd();
const allPaths = [...buildPaths, ...testPaths];

if (!allPaths.length) {
throw new SchematicsException(
'Could not find any tsconfig file. Cannot run the HttpBackend migration.',
);
}

for (const tsconfigPath of allPaths) {
runMigration(tree, tsconfigPath, basePath);
}
};
}

function runMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const program = createMigrationProgram(tree, tsconfigPath, basePath);
const sourceFiles = program
.getSourceFiles()
.filter((sourceFile) => canMigrateFile(basePath, sourceFile, program));

for (const sourceFile of sourceFiles) {
let update: UpdateRecorder | null = null;

const rewriter = (startPos: number, width: number, text: string | null) => {
if (update === null) {
// Lazily initialize update, because most files will not require migration.
update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
}
update.remove(startPos, width);
if (text !== null) {
update.insertLeft(startPos, text);
}
};
migrateFile(sourceFile, rewriter);

if (update !== null) {
tree.commitUpdate(update);
}
}
}
Loading