TB Angular upgrade 20 by cdavalos7 · Pull Request #7141 · tensorflow/tensorboard

cdavalos7 · 2026-07-03T17:13:02Z

Motivation for features / changes

Part of a step-by-step Angular migration from v16 to v22. This PR upgrades the frontend from Angular 19 to Angular 20.

Technical description of changes

Updated all @angular/* packages to ^20.0.0 (resolved 20.3.25 in yarn.lock), @ngrx/* to ^20.0.0, and TypeScript to 5.8.3
Pinned @angular/build-tooling to the Angular 20.0.0 stable commit (b87de6c1) and renamed the patch file to match the new internal version string
Added aspect_rules_js v2.1.0 to WORKSPACE. It is required by the updated build-tools spec-bundling rules, which now use JsInfo providers from that ruleset.
Fix karma web test suite for Angular 20 upgrade
- Updated @io_bazel_rules_webtesting from browsers-0.3.3 to browsers-0.3.4 to upgrade Chromium from Chrome 94 to Chrome 123
- Added custom browser target at tensorboard/tools/browsers since the default alias has restricted visibility and cannot be used from TensorBoard targets
- Changed spec_bundle target to es2021 since Chrome 123 crashes on ES2022 class static blocks emitted by Angular npm packages.
- Skipped WebGL tests with pending() since Chrome 123 headless on macOS ARM64 has no GPU and ThreeRenderer throws on class construction.
- Fixed tests that relied on ng-reflect-* attributes removed in Angular 20, now reading from componentInstance or nativeElement instead.
- Updated formatter test since Chrome 123 ICU library changed the en-US date time separator.

Detailed steps to verify changes work correctly (as executed by you)

Alternate designs / implementations considered (or N/A)

…testing suites

…3.4 to use a newer version of Chrome for testing

karma_web_test_suite from concatjs already defaults to @io_bazel_rules_webtesting//browsers:chromium-local which points to Chrome 123 via browsers-0.3.4 so the custom browser target and json were duplicating what was already there.

arcra · 2026-07-13T17:07:32Z

-          expect(slider.attributes['ng-reflect-min']).toBe('0');
-          expect(slider.attributes['ng-reflect-max']).toBe(
-            String(numExecutions - displayCount)
+          // angular 20 dropped ng-reflect attrs so we read min and max from componentInstance


This is useful context for the PR, but I think we can remove it from the code.

Here and other files where you changed this.

arcra · 2026-07-13T17:14:31Z

-        // Simulate dragging fob to step 10.
+        );
+        const testController = cardFobDebugEl.componentInstance;
+        // rootTop offsets clientY so mousePosition = clientY - rootTop stays


I'm not really following this comment. Can it be clarified or simplified somehow?

What is the "frame of reference" for clientY? Is it with respect to the top of the element? Or the bottom of the element? Or the viewport, or what?

And/or what is rootTop meant to be? Is it an "absolute" position in the viewport, and then clientY too? It seems to be taking the "top" boundary of the card "fob", so maybe the name is confusing.

Maybe instead of rootTop it should be something like fobPosition or startingFobPosition, or something?

I believe this is saying that because the "selection" can only "snap" to existing values, we use 25 to get it "closer to step 10" than "closer to step 5", right?

Perhaps this could be clearer if we say something like this (will likely need to be updated, but the general idea, maybe):

clientY and startingFobPosition are given as absolute measurements from the bottom of the viewport. The step selector will "snap" to the closest option. Step 5 ends up around 24 pixels from startingFobPosition, and step 10 around 27 pixels above, so we set this to a value closer to that step 10.

And then (if this is more or less accurate) I think it would be better to just use 27 for the value, instead of 25?

Thank you good point, I'll clarify it.

clientY is viewport-relative. From top to bottom. That's the standard MouseEvent behavior.

rooTop it's the top of <card-fob-controller> the controller container. It's the same element the component itself uses as its reference point in getMousePositionFromEvent() I will rename it to controllerRootTop

You're right the comment it's about the snapping to an existing step, however is not to the closer steps. The code snaps to the first step it reaches whose axis position the mouse already reached, not the nearest. That's why I picked 25, just needs to land anywhere between step 5 (23.75) and 10 (27.5). I will update the comment to remove closer to.

arcra · 2026-07-13T17:35:07Z

    describe('formatLong', () => {
      it('formats using localization', () => {
        // jasmine + Angular seems to mock out the timezone by default (to UTC).
+        // chrome 123 icu upgrade changed the en-US date time separator from a


Again, this is useful context for the review, but we can remove it from the code.

It should be clear that this is the expected output from whatever formatter we're using, and the historical behavior is not relevant context.

Agree, thank you

arcra · 2026-07-13T17:36:58Z


-    # We use our own browser definition based on the archives defined below, but
-    # this seems to be required by the rules_webtesting dependency.
+    # browser_repositories registers the Chrome 122/123 binaries for all


We can remove this comment too.

cdavalos7 · 2026-07-13T23:26:11Z

-        // Simulate dragging fob to step 10.
+        );
+        const testController = cardFobDebugEl.componentInstance;
+        // rootTop offsets clientY so mousePosition = clientY - rootTop stays


Thank you good point, I'll clarify it.

clientY is viewport-relative. From top to bottom. That's the standard MouseEvent behavior.

rooTop it's the top of <card-fob-controller> the controller container. It's the same element the component itself uses as its reference point in getMousePositionFromEvent() I will rename it to controllerRootTop

You're right the comment it's about the snapping to an existing step, however is not to the closer steps. The code snaps to the first step it reaches whose axis position the mouse already reached, not the nearest. That's why I picked 25, just needs to land anywhere between step 5 (23.75) and 10 (27.5). I will update the comment to remove closer to.

cdavalos7 · 2026-07-13T23:30:01Z

    describe('formatLong', () => {
      it('formats using localization', () => {
        // jasmine + Angular seems to mock out the timezone by default (to UTC).
+        // chrome 123 icu upgrade changed the en-US date time separator from a


Agree, thank you

cdavalos7 · 2026-07-13T23:30:32Z

        expect(
          wallTimeFormatter.formatLong(new Date('2020-1-5 13:23').getTime())
-        ).toBe('January 5, 2020, 1:23:00.000 PM UTC');
+        ).toBe('January 5, 2020 at 1:23:00.000 PM UTC');


jasmine + Angular seems to mock out the timezone by default (to UTC).
Chrome 123 (Angular 20) icu upgrade changed the en-US date time separator from a
omma to the word at so Jan 5 2020 at 1:23 PM instead of comma

cdavalos7 · 2026-07-13T23:34:39Z

+    # override the newer versions when called after browser_repositories.
    browser_repositories(chromium = True)

-    # Chromium browser for multiple platforms, pinned to Chromium 84.0.4147.0.


browser_repositories already registers the Chrome 122 and 123 binaries for all platforms through browsers-0.3.4.bzl. These got removed because calling them after browser_repositories just overrides the newer 122/123 binaries with the old Chrome 84, so they silently downgrading the browser used in tests.

cdavalos7

Done

arcra · 2026-07-14T00:16:04Z

+        const testController = cardFobDebugEl.componentInstance;
+        // Drag the fob from step 0 to step 10.
+        const controllerRootTop =
+          // top of the card fob controller host in viewport coords, same point the component uses internally in getMousePositionFromEvent


Can we add line breaks to these comments (this and others in this change), so they're more easily readable when the text is wrapped around?

Also, I think the comment should be before the variable name too.

arcra · 2026-07-14T00:17:02Z

      });
+      // MatSliderThumb sets the initial value async thru NgModel so tick is
+      // needed before nativeElement.value is ready. also angular 20 dropped
+      // ng-reflect attrs so we read from nativeElement directly


Likewise, we can remove this about the ng-reflect that doesn't exist in the code anymore.

arcra · 2026-07-14T18:29:17Z

        upperValue: 3,
      });
-      // MatSliderThumb sets the initial value async thru NgModel so tick is
-      // needed before nativeElement.value is ready. also angular 20 dropped


Sorry for not being clear. I think the comment related to the "tick" was still worth keeping. Just not the ng-reflect part that is no longer in the code, so it doesn't really seem useful to someone reading the code in the future.

Angular upgrade 20. no sourcecode changes just build changes

97999ae

cdavalos7 changed the title ~~Angular upgrade 20. no sourcecode changes just build changes~~ TB Angular upgrade 20 Jul 3, 2026

Angular upgrade 20: fix tests and updated to use Chromium to 123 for …

a91bf6a

…testing suites

cdavalos7 marked this pull request as ready for review July 10, 2026 00:39

cdavalos7 added 6 commits July 10, 2026 10:41

Upgraded io_bazel_rules_webtesting from browsers-0.3.3 to browsers-0.…

5558a84

…3.4 to use a newer version of Chrome for testing

Removed unused dependencies

ec31705

Fixed failing tests

cac87e0

Fixed unit test failing

967c3a6

Linter failing fixed.

fe2a27b

Remove custom browser files from tools/browsers

56cafad

karma_web_test_suite from concatjs already defaults to @io_bazel_rules_webtesting//browsers:chromium-local which points to Chrome 123 via browsers-0.3.4 so the custom browser target and json were duplicating what was already there.

cdavalos7 requested a review from arcra July 13, 2026 16:53

arcra requested changes Jul 13, 2026

View reviewed changes

cdavalos7 commented Jul 13, 2026

View reviewed changes

Updated with comments from feedback in PR

9e5f690

cdavalos7 commented Jul 13, 2026

View reviewed changes

arcra approved these changes Jul 14, 2026

View reviewed changes

Inline comments clarification

a9017b6

arcra approved these changes Jul 14, 2026

View reviewed changes

Added comment to clarify tick() in tests

8b8cb1e

cdavalos7 merged commit 1b86d2d into tensorflow:master Jul 15, 2026
18 checks passed

Uh oh!

Conversation

cdavalos7 commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for features / changes

Technical description of changes

Detailed steps to verify changes work correctly (as executed by you)

Alternate designs / implementations considered (or N/A)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdavalos7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cdavalos7 commented Jul 3, 2026 •

edited

Loading