docs: Adds HTTP communication guidance to Angular Skills#69645
docs: Adds HTTP communication guidance to Angular Skills#69645SkyZeroZx wants to merge 1 commit into
Conversation
| - Calling `provideHttpClient()` with no features is not required for basic HTTP requests, but it configures the default HTTP feature set for that injector, including Angular's XSRF interceptor. | ||
| - Prefer `provideHttpClient(...)` over `HttpClientModule` for feature configuration, especially with multiple injectors. | ||
| - Use `withRequestsMadeViaParent()` when a child injector should add interceptors while still delegating to the parent HTTP chain. | ||
| - Prefer CORS over JSONP. Use `withJsonpSupport()` only for legacy APIs that require JSONP. |
There was a problem hiding this comment.
In fact, I think we should avoid mentioning or directly say never to use JsonP because of the risk of XSS. Or should we still leave it as legacy support?
There was a problem hiding this comment.
Yeah, let's just remove any mention of JSONP. It belongs in the past.
There was a problem hiding this comment.
Wouldn't it be better to emphasize not using it (I say this to avoid confusion or to provide better guidance for the agents)?
Since, for example, if I start a new project (even with Angular skills) it adds withFetch for some reason when there's an unrelated problem, I would assume this might happen unexpectedly with JsonP
EDIT: According to GTP 5.5 using skill-creator, it indicates that it would be better to omit it, so let's go with that option.
| ``` | ||
|
|
||
| - `HttpClient` uses the fetch backend by default. | ||
| - Use `withXhr()` only when upload progress events are required. Do not use `withXhr()` for server-side rendering. |
There was a problem hiding this comment.
This will be deprecated I think.
There was a problem hiding this comment.
As far as I know, XHR is not going to be deprecated; from my POV, it is still useful and valuable on the client side for upload reporting support.
It is supposedly being deprecated because the xhr2 library is vulnerable, according to
Because it can filter Authorization headers without any way to prevent it in SSR, which is why SSR itself is being deprecated. For more context, this is similar to GHSA-qxh6-94w6-9r5p
There was a problem hiding this comment.
It is unlikely to be deprecated in a near future. Fetch still doesn't support upload progress report.
dominicbachmann
left a comment
dominicbachmann
left a comment
There was a problem hiding this comment.
What do you think about also adding examples of how to test this, as described in https://angular.dev/guide/http/testing?
| import {input} from '@angular/core'; | ||
|
|
||
| export class UserProfile { | ||
| userId = input.required<string>(); |
There was a problem hiding this comment.
The HttpClient example above uses modifiers like private and readonly. Is there a particular reason they're not used here? Otherwise I'd keep the examples consistent
There was a problem hiding this comment.
The HttpClient example above uses modifiers like private and readonly. Is there a particular reason they're not used here? Otherwise I'd keep the examples consistent
I agree, it should be consistent.
What do you think about also adding examples of how to test this, as described in angular.dev/guide/http/testing?
I'm not sure if we should add testing to this guide because we might have context that would consume more tokens for our agent. I believe everything related to testing should be a separate skill, and it would also be called as required by the user prompt.
SkyZeroZx
force-pushed
the
skills/http-skill
branch
from
ad636c9 to
a5372ee
Compare
| - **Component Harnesses**: Standard patterns for robust component interaction. Read [component-harnesses.md](references/component-harnesses.md) | ||
| - **Router Testing**: Using `RouterTestingHarness` for reliable navigation tests. Read [router-testing.md](references/router-testing.md) | ||
| - **End-to-End (E2E) Testing**: Best practices for E2E tests with Cypress. Read [e2e-testing.md](references/e2e-testing.md) | ||
|
|
There was a problem hiding this comment.
@SkyZeroZx What do you think about adding a http testing reference here?
There was a problem hiding this comment.
Yes, that makes sense, but I think it would be better to address it in a follow-up PR to avoid adding extra overhead to this one.
There was a problem hiding this comment.
Personally I would add to this PR. Let's see what the others think.
SkyZeroZx
force-pushed
the
skills/http-skill
branch
from
a5372ee to
8ed3d3d
Compare
4 participants
Closes #68022