Angular upgrade from version 16 to 17

.github/workflows/ci.yml

@@ -310,7 +310,7 @@ jobs:
       - uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
       - uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
         with:
-          node-version: 16
+          node-version: 18
       - run: yarn install --ignore-engines
         # You can run `yarn fix-lint` to fix all Prettier complaints, although at this point this will try to fix too many things.
         # To fix only the files changed in this PR, see the command below.

WORKSPACE

@@ -70,24 +70,37 @@ rules_closure_dependencies(
 
 http_archive(
     name = "build_bazel_rules_nodejs",
-    sha256 = "c29944ba9b0b430aadcaf3bf2570fece6fc5ebfb76df145c6cdad40d65c20811",
+    sha256 = "f02557f31d4110595ca6e93660018bcd7fdfdbe7d0086089308f1b3af3a7a7ee",
     urls = [
-        "http://mirror.tensorflow.org/github.com/bazelbuild/rules_nodejs/releases/download/5.7.0/rules_nodejs-5.7.0.tar.gz",
-        "https://github.com/bazelbuild/rules_nodejs/releases/download/5.7.0/rules_nodejs-5.7.0.tar.gz",
+        "https://github.com/bazelbuild/rules_nodejs/releases/download/5.8.1/rules_nodejs-5.8.1.tar.gz",
     ],
 )
 
 load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")
 
 build_bazel_rules_nodejs_dependencies()
 
-load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
+load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")
+
+# Angular 17 needs Node.js 18.17 or higher. rules_nodejs 5.8.1 does not
+# include Node 18, so we add it here manually.
+# @TODO: We plan to upgrade to a newer version of rules_nodejs that includes Node 18 and remove this manual addition in next version upgrade.
+node_repositories(
+    node_repositories = {
+        "18.20.8-darwin_arm64": ("node-v18.20.8-darwin-arm64.tar.gz", "node-v18.20.8-darwin-arm64", "bae4965d29d29bd32f96364eefbe3bca576a03e917ddbb70b9330d75f2cacd76"),
+        "18.20.8-darwin_amd64": ("node-v18.20.8-darwin-x64.tar.gz", "node-v18.20.8-darwin-x64", "ed2554677188f4afc0d050ecd8bd56effb2572d6518f8da6d40321ede6698509"),
+        "18.20.8-linux_arm64": ("node-v18.20.8-linux-arm64.tar.xz", "node-v18.20.8-linux-arm64", "224e569dbe7b0ea4628ce383d9d482494b57ee040566583f1c54072c86d1116b"),
+        "18.20.8-linux_amd64": ("node-v18.20.8-linux-x64.tar.xz", "node-v18.20.8-linux-x64", "5467ee62d6af1411d46b6a10e3fb5cacc92734dbcef465fea14e7b90993001c9"),
+        "18.20.8-windows_amd64": ("node-v18.20.8-win-x64.zip", "node-v18.20.8-win-x64", "1a1e40260a6facba83636e4cd0ba01eb5bd1386896824b36645afba44857384a"),
+    },
+    node_version = "18.20.8",
+)
 
 yarn_install(
     name = "npm",
     data = [
-        "//patches:@angular+build-tooling+0.0.0-7d103b83a07f132629592fc9918ce17d42a5e382.patch",
-        "//patches:@bazel+concatjs+5.7.0.patch",
+        "//patches:@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch",
+        "//patches:@bazel+concatjs+5.8.1.patch",
     ],
     # "Some rules only work by referencing labels nested inside npm packages
     # and therefore require turning off exports_directories_only."

package.json

@@ -28,18 +28,18 @@
   },
   "homepage": "https://github.com/tensorflow/tensorboard#readme",
   "devDependencies": {
-    "@angular-devkit/build-angular": "^15.2.9",
-    "@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#fb42478534df7d48ec23a6834fea94a776cb89a0",
-    "@angular/cli": "^16.2.0",
-    "@angular/compiler": "16.2.12",
-    "@angular/compiler-cli": "^16.2.12",
+    "@angular-devkit/build-angular": "^17.0.0",
+    "@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#3069be882e3e41cdb3dad58788d878e31d7d82e8",
+    "@angular/cli": "^17.0.0",
+    "@angular/compiler": "17.3.12",
+    "@angular/compiler-cli": "^17.0.0",
     "@babel/core": "^7.16.12",
-    "@bazel/concatjs": "5.7.0",
-    "@bazel/esbuild": "5.7.0",
+    "@bazel/concatjs": "5.8.1",
+    "@bazel/esbuild": "5.8.1",
     "@bazel/ibazel": "^0.15.9",
-    "@bazel/jasmine": "5.7.0",
-    "@bazel/terser": "5.7.0",
-    "@bazel/typescript": "5.7.0",
+    "@bazel/jasmine": "5.8.1",
+    "@bazel/terser": "5.8.1",
+    "@bazel/typescript": "5.8.1",
     "@types/d3": "5.7.2",
     "@types/jasmine": "^3.8.2",
     "@types/lodash": "^4.14.172",
@@ -61,22 +61,22 @@
     "prettier-plugin-organize-imports": "2.3.4",
     "requirejs": "^2.3.7",
     "tslib": "^2.3.0",
-    "typescript": "4.9.5",
+    "typescript": "5.2.2",
     "yarn-deduplicate": "^5.0.0"
   },
   "dependencies": {
-    "@angular/animations": "^16.2.12",
-    "@angular/cdk": "^16.2.14",
-    "@angular/common": "16.2.12",
-    "@angular/core": "^16.2.12",
-    "@angular/forms": "^16.2.12",
-    "@angular/localize": "^16.2.12",
-    "@angular/material": "^16.2.14",
-    "@angular/platform-browser": "^16.2.12",
-    "@angular/platform-browser-dynamic": "^16.2.12",
-    "@angular/router": "^16.2.12",
-    "@ngrx/effects": "^15.4.0",
-    "@ngrx/store": "^15.4.0",
+    "@angular/animations": "^17.0.0",
+    "@angular/cdk": "^17.0.0",
+    "@angular/common": "17.3.12",
+    "@angular/core": "^17.0.0",
+    "@angular/forms": "^17.0.0",
+    "@angular/localize": "^17.0.0",
+    "@angular/material": "^17.0.0",
+    "@angular/platform-browser": "^17.0.0",
+    "@angular/platform-browser-dynamic": "^17.0.0",
+    "@angular/router": "^17.0.0",
+    "@ngrx/effects": "^17.0.0",
+    "@ngrx/store": "^17.0.0",
     "@polymer/decorators": "^3.0.0",
     "@polymer/iron-behaviors": "^3.0.1",
     "@polymer/iron-collapse": "^3.0.1",
@@ -131,7 +131,7 @@
     "three": "~0.137.0",
     "umap-js": "^1.3.2",
     "web-animations-js": "^2.3.2",
-    "zone.js": "^0.13.3"
+    "zone.js": "^0.14.0"
   },
   "resolutions": {
     "@types/d3-brush": "1.1.8",

patches/@angular+build-tooling+0.0.0-7d103b83a07f132629592fc9918ce17d42a5e382.patch → patches/@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch

@@ -1,5 +1,5 @@
 diff --git a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel
-index d5a8645..4b57378 100755
+index 870da1b..3f1e5c5 100755
 --- a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel
 +++ b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel
 @@ -23,6 +23,7 @@ js_library(
@@ -9,24 +9,24 @@ index d5a8645..4b57378 100755
 +        "@npm//@babel/helper-split-export-declaration",
      ],
  )
-
+ 
 diff --git a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs
-index 57cd2b9..2e5eaf1 100755
+index 6d5ec3f..ad4b529 100755
 --- a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs
 +++ b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs
-@@ -43,9 +43,12 @@ export function createEsbuildAngularOptimizePlugin(
-
-         // If the current file is denoted as explicit side effect free, add the pure
-         // top-level functions optimization plugin for this file.
--        if (isSideEffectFreeFn !== null && isSideEffectFreeFn(args.path)) {
--          plugins.push(pureToplevelFunctionsPlugin);
--        }
-+        // For TensorBoard: This plugin aggressively culls symbols in a way that
-+        // is incompatible with TensorBoard source. Remove it. The binary is
-+        // bigger than it otherwise could be but the bundle also happens faster.
-+        //if (isSideEffectFreeFn !== null && isSideEffectFreeFn(args.path)) {
-+        //  plugins.push(pureToplevelFunctionsPlugin);
-+        //}
-
-         const {code} = await babel.transformAsync(content, {
-           filename: filePath,
+@@ -86,11 +86,10 @@ export async function createEsbuildAngularOptimizePlugin(opts, additionalBabelPl
+             devkitOptimizePlugins.adjustTypeScriptEnumsPlugin,
+           );
+
+-          // If the current file is denoted as explicit side effect free, add the pure
+-          // top-level functions optimization plugin for this file.
+-          if (opts.optimize.isSideEffectFree && opts.optimize.isSideEffectFree(args.path)) {
+-            plugins.push(devkitOptimizePlugins.pureToplevelFunctionsPlugin);
+-          }
++          // Disabled: breaks TensorBoard by removing code that is still used.
++          //if (opts.optimize.isSideEffectFree && opts.optimize.isSideEffectFree(args.path)) {
++          //  plugins.push(devkitOptimizePlugins.pureToplevelFunctionsPlugin);
++          //}
+         }
+
+         const shouldRunLinker =

patches/@bazel+concatjs+5.7.0.patch → patches/@bazel+concatjs+5.8.1.patch

@@ -28,24 +28,17 @@ index fed787a..377915a 100755
      return struct(
          closure_js = closure_js_files,
          devmode_js = devmode_js_files,
-diff --git a/node_modules/@bazel/concatjs/web_test/karma.conf.js b/node_modules/@bazel/concatjs/web_test/karma.conf.js
-index 90a03ef..28778c9 100755
---- a/node_modules/@bazel/concatjs/web_test/karma.conf.js
-+++ b/node_modules/@bazel/concatjs/web_test/karma.conf.js
-@@ -384,7 +384,15 @@ try {
-             conf.browsers.push(launcher);
-           } else {
-             const launcher = 'CustomChrome';
--            conf.customLaunchers = {[launcher]: {base: browser, flags: additionalArgs}};
-+            // For TensorBoard: Patch the CustomChrome launcher so that even it
-+            // specifies the --no-sandbox flag. This is to workaround
-+            // incompatibilities with some environments.
-+            //
-+            // Specifically we were seeing errors like:
-+            // [WARNING:gpu_process_host.cc(1228)] The GPU process has crashed 6 time(s)
-+            // [FATAL:gpu_data_manager_impl_private.cc(439)] GPU process isn't usable. Goodbye.
-+            conf.customLaunchers =
-+                {[launcher]: {base: browser, flags: ['--no-sandbox', ...additionalArgs]}};
-             conf.browsers.push(launcher);
-           }
-         }
+diff --git a/node_modules/@bazel/concatjs/package.json b/node_modules/@bazel/concatjs/package.json
+index 1234567..abcdefg 100755
+--- a/node_modules/@bazel/concatjs/package.json
++++ b/node_modules/@bazel/concatjs/package.json
+@@ -24,7 +24,8 @@
+     "dependencies": {
+         "protobufjs": "6.8.8",
+         "source-map-support": "0.5.9",
+-        "tsutils": "3.21.0"
++        "tsutils": "3.21.0",
++        "typescript": "5.2.2"
+     },
+     "peerDependencies": {
+         "karma": ">=4.0.0",

tensorboard/defs/defs.bzl

@@ -179,7 +179,7 @@ def tf_ts_library(srcs = [], strict_checks = True, **kwargs):
         **kwargs
     )
 
-def tf_ng_web_test_suite(name, deps = [], **kwargs):
+def tf_ng_web_test_suite(name, deps = [], external = [], **kwargs):
     """TensorBoard wrapper for the rule for a Karma web test suite.
 
     This uses the Angular team's internal toolchain for bundling
@@ -218,6 +218,7 @@ def tf_ng_web_test_suite(name, deps = [], **kwargs):
         workspace_name = "org_tensorflow_tensorboard",
         run_angular_linker = False,
         platform = "browser",
+        external = external,
     )
 
     karma_web_test_suite(

tensorboard/plugins/projector/vz_projector/BUILD

@@ -140,6 +140,19 @@ tf_ts_library(
 
 tf_ng_web_test_suite(
     name = "vz_projector_test",
+    # Exclude Node.js-only packages from the browser bundle.
+    # These come from @tensorflow/tfjs-core -> node-fetch.
+    external = [
+        "node-fetch",
+        "stream",
+        "http",
+        "https",
+        "url",
+        "zlib",
+        "buffer",
+        "punycode",
+        "string_decoder",
+    ],
     deps = [
         ":vz_projector_test_lib",
     ],

tensorboard/tools/whitespace_hygiene_test.py

@@ -25,7 +25,14 @@
 import sys
 
 
-exceptions = frozenset([])
+# @TODO: Remove this exception when the patch file is no longer needed.
+# Patch files use a trailing space on blank lines to mark them as context.
+# This is required by patch-package and cannot be removed.
+exceptions = frozenset(
+    [
+        "patches/@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch",
+    ]
+)
 
 
 @dataclasses.dataclass(frozen=True)

tensorboard/webapp/alert/views/alert_snackbar_test.ts

@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
 import {OverlayContainer} from '@angular/cdk/overlay';
-import {TestBed} from '@angular/core/testing';
+import {fakeAsync, flush, TestBed} from '@angular/core/testing';
 import {MatButtonModule} from '@angular/material/button';
 import {MatSnackBar, MatSnackBarModule} from '@angular/material/snack-bar';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
@@ -130,7 +130,7 @@ describe('alert snackbar', () => {
     });
   });
 
-  it('closes the snackbar on click', async () => {
+  it('closes the snackbar on click', fakeAsync(() => {
     const fixture = TestBed.createComponent(AlertSnackbarContainer);
     fixture.detectChanges();
     store.overrideSelector(selectors.getLatestAlert, {
@@ -144,14 +144,13 @@ describe('alert snackbar', () => {
       .querySelector(Selectors.DISMISS_BUTTON);
     expect(dismissEl).toBeTruthy();
     (dismissEl as HTMLButtonElement).click();
-    fixture.detectChanges();
-    await fixture.whenStable();
+    flush();
 
     const snackbarAfterEl = overlayContainer
       .getContainerElement()
       .querySelector(Selectors.SNACKBAR);
     expect(snackbarAfterEl).not.toBeTruthy();
-  });
+  }));
 
   it('shows the followup action if needed', () => {
     const fixture = TestBed.createComponent(AlertSnackbarContainer);
@@ -188,7 +187,7 @@ describe('alert snackbar', () => {
     expect(followupEl).not.toBeTruthy();
   });
 
-  it('dispatches a followup action and closes', async () => {
+  it('dispatches a followup action and closes', fakeAsync(() => {
     const fixture = TestBed.createComponent(AlertSnackbarContainer);
     fixture.detectChanges();
     store.overrideSelector(selectors.getLatestAlert, {
@@ -205,17 +204,16 @@ describe('alert snackbar', () => {
       .getContainerElement()
       .querySelector(Selectors.FOLLOWUP_BUTTON);
     (followupEl as HTMLButtonElement).click();
-    fixture.detectChanges();
-    await fixture.whenStable();
+    flush();
 
     expect(recordedActions).toEqual([testAction()]);
     const snackbarAfterEl = overlayContainer
       .getContainerElement()
       .querySelector(Selectors.SNACKBAR);
     expect(snackbarAfterEl).not.toBeTruthy();
-  });
+  }));
 
-  it('dispatches a followup action with payload and closes', async () => {
+  it('dispatches a followup action with payload and closes', fakeAsync(() => {
     const fixture = TestBed.createComponent(AlertSnackbarContainer);
     fixture.detectChanges();
     store.overrideSelector(selectors.getLatestAlert, {
@@ -232,13 +230,12 @@ describe('alert snackbar', () => {
       .getContainerElement()
       .querySelector(Selectors.FOLLOWUP_BUTTON);
     (followupEl as HTMLButtonElement).click();
-    fixture.detectChanges();
-    await fixture.whenStable();
+    flush();
 
     expect(recordedActions).toEqual([testActionWithProps({foo: true})]);
     const snackbarAfterEl = overlayContainer
       .getContainerElement()
       .querySelector(Selectors.SNACKBAR);
     expect(snackbarAfterEl).not.toBeTruthy();
-  });
+  }));
 });

tensorboard/webapp/theme/_tb_theme.template.scss

@@ -271,15 +271,10 @@ $tb-dark-theme: map_merge(
 
 // Apply themed style for the global stylesheet (styles.scss).
 @mixin tb-global-themed-styles() {
-  // TODO(JamesHollyer): remove legacy component themes once all components
-  // are migrated.
   @include mat.core();
-  @include mat.all-legacy-component-typographies();
   @include mat.all-component-typographies();
   // Include all theme-styles for the components based on the current theme.
   @include mat.all-component-themes($tb-theme);
-  @include mat.all-legacy-component-themes($tb-theme);
-  @include mat.all-component-themes($tb-theme);
 
   body {
     // Prevents ngx-color-picker from creating a scrollbar and misposition.
@@ -370,9 +365,6 @@ $tb-dark-theme: map_merge(
       }
     }
 
-    // TODO(JamesHollyer): remove legacy component themes once all components
-    // are migrated.
     @include mat.all-component-themes($tb-dark-theme);
-    @include mat.all-legacy-component-themes($tb-dark-theme);
   }
 }