profile
viewpoint
Alan Agius alan-agius4 @google Switzerland Angular Team, Loves Fishing, Food, Beer & Coffee 🇲🇹 🇨🇭

alan-agius4/angular-benchpress-playground 4

Angular Benchpress Playground

alan-agius4/analyse 0

analyse web app for webpack stats

alan-agius4/angular 0

The modern web developer’s platform

alan-agius4/angular-aot-metadata-bug 0

Repo for Angular AOT Metadata Bug

alan-agius4/angular-cli 0

CLI tool for Angular

alan-agius4/angular-cli-ghpages 0

🚀 Deploy your 🅰️Angular app to GitHub pages directly from the Angular CLI! Available on NPM.

alan-agius4/angular-eslint 0

:sparkles: Monorepo for all the tooling related to using ESLint with Angular

PullRequestReviewEvent

delete branch alan-agius4/angular-cli

delete branch : hmr-server-collision-warn

delete time in a day

push eventangular/angular-cli

Alan Agius

commit sha 2d2e79921a72c4fafad673abe501ba10400403d2

fix(@angular-devkit/build-angular): clean up internal Angular state during rendering SSR This commit clean ups the compiled components state when the build is being executed in watch mode. This is required as otherwise during development `Component ID generation collision detected` are displayed on the server. Closes: #25924

view details

push time in a day

issue closedangular/angular-cli

Component ID generation collision detected is thrown after first change when running default angular installation

Command

serve

Is this a regression?

  • [ ] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

Component ID generation collision detected is thrown after first change when running default angular installation

Minimal Reproduction

  1. npm create @angular@next
  2. Choose ssr to be enabled
  3. ng serve
  4. change app component html -> no warning
  5. change app component html again -> warning is shown in console

CleanShot 2023-09-29 at 11 56 57@2x

Exception or Error

No response

Your Environment

Angular CLI: 17.0.0-next.6
Node: 18.17.0
Package Manager: npm 9.6.7
OS: darwin arm64

Angular: 17.0.0-next.6
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, platform-server
... router, ssr

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.0-next.6
@angular-devkit/build-angular   17.0.0-next.6
@angular-devkit/core            17.0.0-next.6
@angular-devkit/schematics      17.0.0-next.6
@schematics/angular             17.0.0-next.6
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.0

Anything else relevant?

No response

closed time in a day

eneajaho

PR merged angular/angular-cli

fix(@angular-devkit/build-angular): clean up internal Angular state during rendering SSR action: merge target: minor

This commit clean ups the compiled components state when the build is being executed in watch mode. This is required as otherwise during development Component ID generation collision detected are displayed on the server.

Closes: #25924

+23 -3

0 comment

3 changed files

alan-agius4

pr closed time in a day

pull request commentangular/angular-cli

test(@angular/cli): remove Safari 15 test

Safari 15 is still support but not by default. Users needs to opt-in by adding it in their browserslist config.

alan-agius4

comment created time in a day

delete branch alan-agius4/domino

delete branch : attribute-name-serialize

delete time in a day

push eventangular/domino

Alan Agius

commit sha 9eea7b48988ff4ec053cc7cde964729ff9e20114

fix: escape angle brackets in attribute value Angle brackets inside attribute values are now escaped, This is to ensure unsafe behaviour after de-serialization. This also is done in Chrome see: https://chromestatus.com/feature/5083926074228736

view details

Alan Agius

commit sha 77f745f670cc18b33eca4ba60a18c98e61a5a148

fix: santize noscript contents Previously, `noscript` was parsed as raw text. This however is not correct as `noscript` can contain child elements. We no updated the logic to parse noscript as an element which causes santization of child nodes.

view details

push time in a day

PR merged angular/domino

fix: escape angel brackets in attribute value

fix: santize noscript contents

Previously, noscript was parsed as raw text. This however is not correct as noscript can contain child elements. We no updated the logic to parse noscript as an element which causes santization of child nodes.

fix: escape angle brackets in attribute value

Angle brackets inside attribute values are now escaped, This is to ensure unsafe behaviour after de-serialization.

+68 -35

0 comment

5 changed files

alan-agius4

pr closed time in a day

push eventalan-agius4/domino

Alan Agius

commit sha d420c8c078ef8ce6d3bc8013412c68c310f662a0

fix: santize noscript contents Previously, `noscript` was parsed as raw text. This however is not correct as `noscript` can contain child elements. We no updated the logic to parse noscript as an element which causes santization of child nodes.

view details

push time in a day

push eventalan-agius4/domino

Alan Agius

commit sha bf9de987eda32d62f94d89e014e87c08eb1ecdc2

Update test/xss.js Co-authored-by: Andrew Kushnir <43554145+AndrewKushnir@users.noreply.github.com>

view details

push time in a day

PullRequestReviewEvent

Pull request review commentangular/domino

fix: escape angel brackets in attribute value

 exports.fp170_37 = function() {     '<p><svg><style>*{font-family:\'&lt;/style&gt;&lt;img/src=x\tonerror=xss()//\'}</style></svg></p>'   ); };++exports.escapeAngleBracketsInDivAttr = function() {+  var document = domino.createDocument(+    `<div>You don't have JS! Click<a href="#" title="Search for </div><script>alert(1)</script> without JS">here</a> to go to the no-js website.</div>`+  );+  document.body.innerHTML.should.equal(+    `<div>You don't have JS! Click<a href="#" title="Search for &lt;/div&gt;&lt;script&gt;alert(1)&lt;/script&gt; without JS">here</a> to go to the no-js website.</div>`+  );+};++exports.escapeAngleBracketsInNoScriptAttr = function() {+  debugger;

Good catch

alan-agius4

comment created time in a day

Pull request review commentangular/angular

feat: allow customization of the HttpTransferCache.

 export class HttpRequest<T> {    */   readonly urlWithParams: string; +  /**+   * This property accepts either a boolean to enable/disable transferring cache for eligible+   * requests performed using `HttpClient`, or an object, which allows to configure cache+   * parameters, such as which headers should be included (no headers are included by default).+   *+   * Setting this property will override the options passed to `provideClientHydration()` for this+   * particular request+   */

Something like

  /**
   * Constructs a `POST` request that interprets the body as JSON
   * and returns the response body as an object parsed from JSON.
   *
   * @param url The endpoint URL.
   * @param body The content to replace with.
   * @param options HTTP options
   *
   * @return An `Observable` of the response, with the response body as an object parsed from JSON.
   */
  post(url: string, body: any|null, options?: {
    headers?: HttpHeaders|{[header: string]: string | string[]},
    context?: HttpContext,
    observe?: 'body',
    params?: HttpParams|
          {[param: string]: string | number | boolean | ReadonlyArray<string|number|boolean>},
    reportProgress?: boolean,
    responseType?: 'json',
    withCredentials?: boolean,
    /** Enable .... */
    transferCache?: {
      /** Specify ... */
      includeHeaders?: string[]
    }|boolean

  }): Observable<Object>;
JeanMeche

comment created time in a day

PullRequestReviewEvent
PullRequestReviewEvent

pull request commentangular/angular

feat: allow customization of the HttpTransferCache.

@JeanMeche, can you please use fixup commits?As it’s easier to see what changed after a review. As otherwise, I will have to review the whole thing again.

JeanMeche

comment created time in a day

Pull request review commentangular/angular

feat: allow customization of the HttpTransferCache.

 export class HttpRequest<T> {    */   readonly urlWithParams: string; +  /**+   * This property accepts either a boolean to enable/disable transferring cache for eligible+   * requests performed using `HttpClient`, or an object, which allows to configure cache+   * parameters, such as which headers should be included (no headers are included by default).+   *+   * Setting this property will override the options passed to `provideClientHydration()` for this+   * particular request+   */

You need to do the docs, as part of the inlined typed in the init.

Probably would make sense to have a shared interface to avoid duplicating comments.

Can create an example later as I am currently commuting.

JeanMeche

comment created time in a day

PullRequestReviewEvent

Pull request review commentangular/angular

feat: allow customization of the HttpTransferCache.

 export class HttpRequest<T> {    */   readonly urlWithParams: string; +  /**+   * This property accepts either a boolean to enable/disable transferring cache for eligible+   * requests performed using `HttpClient`, or an object, which allows to configure cache+   * parameters, such as which headers should be included (no headers are included by default).+   *+   * Setting this property will override the options passed to `provideClientHydration()` for this+   * particular request+   */+  readonly transferCache?: {includeHeaders?: string[]}|boolean;

I am not really understand the point about PUT and DELETE. I am asking solely POST.

If on a POST request, transferCache is set enabled, it will still not be cached without the global flag enabled, but this means that now users would need to find all other POST requests that don’t/shouldn’t be cache and disable caching for them.

IMHO, if caching is enabled on a POST request, the global flag doesn’t need to be set to enable caching. At least, I find it more intuitive from a user POV.

//cc @AndrewKushnir

JeanMeche

comment created time in a day

PullRequestReviewEvent

Pull request review commentangular/angular

feat: allow customization of the HttpTransferCache.

 export function withHttpTransferCache(): Provider[] {     }   ]; }+++/**+ * This function will add a proxy to an HttpHeader to intercept calls to get/has+ * and log a warning if the header entry requested has been removed+ */+function appendMissingHeadersDetection(+    url: string, headers: HttpHeaders, headersToInclude: string[]): HttpHeaders {+  let shownUsageWarning = false;+  return new Proxy<HttpHeaders>(headers, {+    get(target: HttpHeaders, prop: keyof HttpHeaders): unknown {+      const value = Reflect.get(target, prop);+      const methods: (keyof HttpHeaders)[] = ['get', 'has'];

Umm.. in that case though getAll should still be logged as this method doesn’t return all headers but rather all the values of a specific header.

JeanMeche

comment created time in a day

PullRequestReviewEvent

push eventangular/angular-cli

cexbrayat

commit sha 0368b23f2e5d8ca9c6191a2db956dc6850daebfc

fix(@schematics/angular): use @types/node v18 Currently, running a schematics that adds `@types/node` to a project, like `ng add @angular/ssr`, will add the v16 types. As nodejs v16 is now EOL, this PR updates the types to v18.

view details

push time in a day

PR merged angular/angular-cli

fix(@schematics/angular): use @types/node v18 target: major action: merge

PR Checklist

Please check to confirm your PR fulfills the following requirements:

<!-- Please check all that apply using "x". -->

  • [x] The commit message follows our guidelines: https://github.com/angular/angular-cli/blob/main/CONTRIBUTING.md#-commit-message-guidelines
  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

<!-- Please check the one that applies to this PR using "x". -->

  • [x] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Currently, running a schematics that adds @types/node to a project, like ng add @angular/ssr, will add the v16 types.

What is the new behavior?

As nodejs v16 is now EOL, this PR updates the types to v18.

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->

Other information

+3 -3

0 comment

3 changed files

cexbrayat

pr closed time in a day

PullRequestReviewEvent
PullRequestReviewEvent

push eventalan-agius4/angular-cli

Alan Agius

commit sha 59a4b009fca577e9d481d171f627daacb95ad8f1

test(@angular/cli): remove Safari 15 test Safari v15 is longer included in the default browserlist configuration of CLI apps.

view details

Alan Agius

commit sha 727066c77fec06cd8aeec77615afb9dbf4cd50b2

fix(@angular-devkit/build-angular): clean up internal Angular state during rendering SSR This commit clean ups the compiled components state when the build is being executed in watch mode. This is required as otherwise during development `Component ID generation collision detected` are displayed on the server. Closes: #25924

view details

push time in a day

push eventangular/angular-cli

Alan Agius

commit sha 0201061935dd0bb8f93c34092d78be47568e5444

test(@angular/cli): remove Safari 15 test Safari v15 is longer included in the default browserlist configuration of CLI apps. (cherry picked from commit 59a4b009fca577e9d481d171f627daacb95ad8f1)

view details

push time in a day

delete branch alan-agius4/angular-cli

delete branch : safari-test

delete time in a day

more