Http/standalone/independent #47502

alxhub · 2022-09-21T01:20:44Z

New design using the with* feature pattern for provideHttp
No providedInRoot fallback - it's a weird half-working state
Immutable interceptor sets, with no inheritance by default

AndrewKushnir

@alxhub looks great 👍

I've just left some cosmetic comments, but I will take another look tomorrow as well.

packages/common/http/public_api.ts

packages/common/http/src/backend.ts

packages/common/http/src/interceptor.ts

packages/common/http/src/provider.ts

AndrewKushnir

Just took another look and the changes look really great, thanks @alxhub 👍 As you mentioned, the API docs are missing atm, but we can also do that in a followup PR (anytime after feature freeze) and I'll be happy to review the docs changes once they are available.

I've left minor comments and the only question I wanted to bring up is around provideHttpClientTesting function.

packages/common/http/src/provider.ts

packages/common/http/src/xsrf.ts

packages/common/http/test/provider_spec.ts

packages/common/http/src/backend.ts

pkozlowski-opensource · 2022-10-05T08:36:15Z

packages/common/http/src/module.ts

@@ -156,7 +111,7 @@ export class HttpClientXsrfModule {
   */
  providers: [
    HttpClient,
-    {provide: HttpHandler, useClass: HttpInterceptingHandler},
+    {provide: HttpHandler, useClass: HttpInterceptorHandler},


note: from what I understand this rename causes failures in G3 as there are applications importing this private symbol.

pkozlowski-opensource · 2022-10-05T09:27:37Z

packages/common/http/test/provider_spec.ts

+
+      TestBed.inject(HttpClient).get('/test', {responseType: 'text'}).subscribe();
+      const req = TestBed.inject(HttpTestingController).expectOne('/test');
+      expect(req.request.headers.get('X-Tag')).toEqual('alpha,beta');


question: is this test enough to illustrate the order of "interceptor wrapping"?

I guess I would like to see something more explicit, that clearly shows how one interceptor can build on top of processing done of another interceptor (we could do this be explicitly "wrapping" a header set by one intereceptor into another string or something of this nature).

This commit introduces the main components of the `provideHttpClient()` provider API, designed in the style of `provideRouter()`. Initial features are defined for including legacy class-based interceptors, JSONP support, and configuring or disabling the builtin XSRF protection. This API is an alternative to providing `HttpClient` via the `HttpClientModule`, and is more tree-shakable and more capable than the NgModule implementation. Tests are included to validate the new configuration format as well as the interoperability of the two styles of providing and configuring `HttpClient`.

This commit converts `HttpClientModule` to use `provideHttpClient()` internally, with a particular configuration of features. Other NgModules related to configuring `HttpClient` are also converted to use the providers directly from various features, to ensure consistency of behavior.

This commit introduces a new feature for `provideHttpClient` called `withInterceptors`. This feature exposes and configures the new concept of functional interceptors. Functional interceptors use functions instead of classes to implement an HTTP interceptor. Such interceptor functions have access to the DI context from the `EnvironmentInjector` in which they're configured via the `inject()` function. Otherwise, functional interceptors are identical in capability to the existing interceptor system.

Ordinarily, providing `HttpClient` (either via `provideHttpClient` or the `HttpClientModule`) creates an entirely separate HTTP context. Requests made via that client are not passed through the interceptor chains that are configured in a parent injector, for this example. This commit introduces a new option for `provideHttpClient` called `withRequestsMadeViaParent()`. When this option is passed, requests made in the child context flow through any injectors, etc. and are then handed off to the parent context. This addresses a longstanding issue with interceptors where it's not possible to extend the set of interceptors in a child context without repeating all of the interceptors from the parent.

This commit deletes a sentence from the `HttpClientJsonpModule` docs which was accidentally copy-pasted from the docs for another symbol.

pkozlowski-opensource

LGTM!

Reviewed-for: public-api

AndrewKushnir

Reviewed-for: public-api

jessicajaniuk · 2022-10-06T19:02:48Z

This PR was merged into the repository by commit c09c1bb.

This commit introduces a new DI token for the set of functional interceptors. This is a no-op in terms of behavior currently, but will allow for the deduplication of the bridge interceptor which connects legacy class- based interceptors to the functional interceptor chain. PR Close #47502

…#47502) This commit rewrites the JSONP interceptor to use the functional interceptor style internally, while still maintaining the same public API and behavior. PR Close #47502

…#47502) @angular/common/http has XSRF protection which is enabled by default and is implemented as an interceptor. Previously, this protection could be disabled with an API which would internally provide a `NoopInterceptor` in place of the standard XSRF interceptor. To achieve the same capability of disabling the XSRF interceptor after it is converted to the functional style, an InjectionToken is added in this commit which disables the XSRF interceptor. This way, the interceptor can be disabled in place without needing to override it via DI (which is difficult for functional interceptors). PR Close #47502

This commit switches the XSRF configuration tokens (for header name and cookie name) to be `providedIn: 'root'`. This is a no-op change now as they are always provided along with any usage of them via `HttpClientModule`, but will become load-bearing as the `provideHttpClient` API will not provide these tokens, and will rely on injecting them from either the parent context or from these root providers. PR Close #47502

) This commit converts the XSRF interceptor into a functional interceptor instead of a legacy class-based interceptor. PR Close #47502

…47502) This commit introduces the `provideHttpClientTesting()` function as an alternative to the `HttpClientTestingModule` (in fact, the NgModule is converted internally to just use the new provider function). PR Close #47502

This commit introduces the main components of the `provideHttpClient()` provider API, designed in the style of `provideRouter()`. Initial features are defined for including legacy class-based interceptors, JSONP support, and configuring or disabling the builtin XSRF protection. This API is an alternative to providing `HttpClient` via the `HttpClientModule`, and is more tree-shakable and more capable than the NgModule implementation. Tests are included to validate the new configuration format as well as the interoperability of the two styles of providing and configuring `HttpClient`. PR Close #47502

…ly (#47502) This commit converts `HttpClientModule` to use `provideHttpClient()` internally, with a particular configuration of features. Other NgModules related to configuring `HttpClient` are also converted to use the providers directly from various features, to ensure consistency of behavior. PR Close #47502

This commit introduces a new feature for `provideHttpClient` called `withInterceptors`. This feature exposes and configures the new concept of functional interceptors. Functional interceptors use functions instead of classes to implement an HTTP interceptor. Such interceptor functions have access to the DI context from the `EnvironmentInjector` in which they're configured via the `inject()` function. Otherwise, functional interceptors are identical in capability to the existing interceptor system. PR Close #47502

) Ordinarily, providing `HttpClient` (either via `provideHttpClient` or the `HttpClientModule`) creates an entirely separate HTTP context. Requests made via that client are not passed through the interceptor chains that are configured in a parent injector, for this example. This commit introduces a new option for `provideHttpClient` called `withRequestsMadeViaParent()`. When this option is passed, requests made in the child context flow through any injectors, etc. and are then handed off to the parent context. This addresses a longstanding issue with interceptors where it's not possible to extend the set of interceptors in a child context without repeating all of the interceptors from the parent. PR Close #47502

This commit deletes a sentence from the `HttpClientJsonpModule` docs which was accidentally copy-pasted from the docs for another symbol. PR Close #47502

angular-automatic-lock-bot · 2022-11-06T16:06:25Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

angular-robot bot added the comp: docs label Sep 21, 2022

ngbot bot added this to the Backlog milestone Sep 21, 2022

alxhub force-pushed the http/standalone/independent branch from 6d980cc to 8ba78ef Compare September 21, 2022 16:39

angular-robot bot added the comp: docs label Sep 21, 2022

alxhub modified the milestones: Backlog, v15 feature freeze blockers Sep 29, 2022

alxhub force-pushed the http/standalone/independent branch from 8ba78ef to c5bbb5b Compare October 3, 2022 23:37

angular-robot bot added the feature Issue that requests a new feature label Oct 3, 2022

alxhub force-pushed the http/standalone/independent branch from c5bbb5b to e6ac0e2 Compare October 4, 2022 00:46

alxhub added the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 4, 2022

alxhub marked this pull request as ready for review October 4, 2022 00:46

pullapprove bot requested review from atscott, dylhunn and pkozlowski-opensource October 4, 2022 00:46

AndrewKushnir reviewed Oct 4, 2022

View reviewed changes

packages/common/http/public_api.ts Outdated Show resolved Hide resolved

packages/common/http/src/backend.ts Outdated Show resolved Hide resolved

packages/common/http/src/interceptor.ts Show resolved Hide resolved

packages/common/http/src/provider.ts Outdated Show resolved Hide resolved

alxhub force-pushed the http/standalone/independent branch from e6ac0e2 to c91103d Compare October 4, 2022 19:02

AndrewKushnir reviewed Oct 4, 2022

View reviewed changes

packages/common/http/src/provider.ts Outdated Show resolved Hide resolved

packages/common/http/src/xsrf.ts Outdated Show resolved Hide resolved

packages/common/http/test/provider_spec.ts Show resolved Hide resolved

packages/common/http/test/provider_spec.ts Show resolved Hide resolved

alxhub force-pushed the http/standalone/independent branch from c91103d to 56edb30 Compare October 4, 2022 21:25

alxhub added the target: major This PR is targeted for the next major release label Oct 4, 2022

pkozlowski-opensource reviewed Oct 5, 2022

View reviewed changes

packages/common/http/src/backend.ts Show resolved Hide resolved

pullapprove bot requested a review from pkozlowski-opensource October 5, 2022 08:18

pkozlowski-opensource reviewed Oct 5, 2022

View reviewed changes

pullapprove bot requested a review from pkozlowski-opensource October 5, 2022 08:36

pkozlowski-opensource reviewed Oct 5, 2022

View reviewed changes

pullapprove bot requested a review from pkozlowski-opensource October 5, 2022 09:27

alxhub added 5 commits October 5, 2022 13:57

docs(http): fix copy-paste error in HttpClientJsonpModule docs

ac2c6fd

This commit deletes a sentence from the `HttpClientJsonpModule` docs which was accidentally copy-pasted from the docs for another symbol.

alxhub force-pushed the http/standalone/independent branch from f3f4497 to ac2c6fd Compare October 5, 2022 20:58

pkozlowski-opensource approved these changes Oct 6, 2022

View reviewed changes

pullapprove bot requested a review from jessicajaniuk October 6, 2022 07:51

pkozlowski-opensource approved these changes Oct 6, 2022

View reviewed changes

AndrewKushnir approved these changes Oct 6, 2022

View reviewed changes

pullapprove bot removed the request for review from atscott October 6, 2022 17:30

jessicajaniuk removed request for dylhunn and jessicajaniuk October 6, 2022 18:17

jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 6, 2022

jessicajaniuk closed this in d7683a6 Oct 6, 2022

jessicajaniuk pushed a commit that referenced this pull request Oct 6, 2022

refactor(http): convert the XSRF interceptor to functional style (#47502

67ef8b1

) This commit converts the XSRF interceptor into a functional interceptor instead of a legacy class-based interceptor. PR Close #47502

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Http/standalone/independent #47502

Http/standalone/independent #47502

alxhub commented Sep 21, 2022

AndrewKushnir left a comment

AndrewKushnir left a comment

pkozlowski-opensource Oct 5, 2022

alxhub Oct 5, 2022

pkozlowski-opensource Oct 5, 2022

pkozlowski-opensource left a comment

AndrewKushnir left a comment

jessicajaniuk commented Oct 6, 2022

angular-automatic-lock-bot bot commented Nov 6, 2022

Http/standalone/independent #47502

Http/standalone/independent #47502

Conversation

alxhub commented Sep 21, 2022

AndrewKushnir left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

pkozlowski-opensource Oct 5, 2022

Choose a reason for hiding this comment

alxhub Oct 5, 2022

Choose a reason for hiding this comment

pkozlowski-opensource Oct 5, 2022

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

jessicajaniuk commented Oct 6, 2022

angular-automatic-lock-bot bot commented Nov 6, 2022