Skip to content

Commit af354e9

Browse files
authored
Angular upgrade from version 16 to 17 (#7085)
## Motivation for features / changes This PR is the second step in a planned major Angular upgrade cycle, where each major version will be delivered in a separate PR, incrementally progressing until the project reaches Angular 21. This specific PR upgrades TensorBoard from Angular 16 to Angular 17 ## Technical description of changes - Upgrade Angular, NgRx, and Material from v16 to v17 - Upgrade TypeScript to 5.2.2 and zone.js to 0.14 - Use Node.js 18 in Bazel and CI (required by Angular 17) - Update build-tooling patch to match Angular 17 version - Fix Bazel TypeScript compiler crash caused by TypeScript 5.x - Fix 3 snackbar tests that broke due to Material 17 behavior change - Remove deprecated Material legacy theme APIs ## Screenshots of UI changes (or N/A) ## Detailed steps to verify changes work correctly (as executed by you) - All 2022 webapp tests pass - yarn lint passes with Node 18 - App builds and runs locally ## Alternate designs / implementations considered (or N/A)
1 parent 1dd4570 commit af354e9

13 files changed

Lines changed: 3703 additions & 2995 deletions

.github/workflows/ci.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,8 @@ jobs:
310310
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
311311
- uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0
312312
with:
313-
node-version: 16
313+
# Angular v17 supports node.js versions: v18.13.0 and newer
314+
node-version: 18
314315
- run: yarn install --ignore-engines
315316
# You can run `yarn fix-lint` to fix all Prettier complaints, although at this point this will try to fix too many things.
316317
# To fix only the files changed in this PR, see the command below.

WORKSPACE

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,24 +70,37 @@ rules_closure_dependencies(
7070

7171
http_archive(
7272
name = "build_bazel_rules_nodejs",
73-
sha256 = "c29944ba9b0b430aadcaf3bf2570fece6fc5ebfb76df145c6cdad40d65c20811",
73+
sha256 = "f02557f31d4110595ca6e93660018bcd7fdfdbe7d0086089308f1b3af3a7a7ee",
7474
urls = [
75-
"http://mirror.tensorflow.org/github.com/bazelbuild/rules_nodejs/releases/download/5.7.0/rules_nodejs-5.7.0.tar.gz",
76-
"https://github.com/bazelbuild/rules_nodejs/releases/download/5.7.0/rules_nodejs-5.7.0.tar.gz",
75+
"https://github.com/bazelbuild/rules_nodejs/releases/download/5.8.1/rules_nodejs-5.8.1.tar.gz",
7776
],
7877
)
7978

8079
load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies")
8180

8281
build_bazel_rules_nodejs_dependencies()
8382

84-
load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
83+
load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")
84+
85+
# Angular 17 needs Node.js 18.17 or higher. rules_nodejs 5.8.1 does not
86+
# include Node 18, so we add it here manually.
87+
# @TODO(@cdavalos7): We plan to upgrade to a newer version of rules_nodejs that includes Node 18 and remove this manual addition in next version upgrade.
88+
node_repositories(
89+
node_repositories = {
90+
"18.20.8-darwin_arm64": ("node-v18.20.8-darwin-arm64.tar.gz", "node-v18.20.8-darwin-arm64", "bae4965d29d29bd32f96364eefbe3bca576a03e917ddbb70b9330d75f2cacd76"),
91+
"18.20.8-darwin_amd64": ("node-v18.20.8-darwin-x64.tar.gz", "node-v18.20.8-darwin-x64", "ed2554677188f4afc0d050ecd8bd56effb2572d6518f8da6d40321ede6698509"),
92+
"18.20.8-linux_arm64": ("node-v18.20.8-linux-arm64.tar.xz", "node-v18.20.8-linux-arm64", "224e569dbe7b0ea4628ce383d9d482494b57ee040566583f1c54072c86d1116b"),
93+
"18.20.8-linux_amd64": ("node-v18.20.8-linux-x64.tar.xz", "node-v18.20.8-linux-x64", "5467ee62d6af1411d46b6a10e3fb5cacc92734dbcef465fea14e7b90993001c9"),
94+
"18.20.8-windows_amd64": ("node-v18.20.8-win-x64.zip", "node-v18.20.8-win-x64", "1a1e40260a6facba83636e4cd0ba01eb5bd1386896824b36645afba44857384a"),
95+
},
96+
node_version = "18.20.8",
97+
)
8598

8699
yarn_install(
87100
name = "npm",
88101
data = [
89-
"//patches:@angular+build-tooling+0.0.0-7d103b83a07f132629592fc9918ce17d42a5e382.patch",
90-
"//patches:@bazel+concatjs+5.7.0.patch",
102+
"//patches:@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch",
103+
"//patches:@bazel+concatjs+5.8.1.patch",
91104
],
92105
# "Some rules only work by referencing labels nested inside npm packages
93106
# and therefore require turning off exports_directories_only."

package.json

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,18 @@
2828
},
2929
"homepage": "https://github.com/tensorflow/tensorboard#readme",
3030
"devDependencies": {
31-
"@angular-devkit/build-angular": "^15.2.9",
32-
"@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#fb42478534df7d48ec23a6834fea94a776cb89a0",
33-
"@angular/cli": "^16.2.0",
34-
"@angular/compiler": "16.2.12",
35-
"@angular/compiler-cli": "^16.2.12",
31+
"@angular-devkit/build-angular": "^17.0.0",
32+
"@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#3069be882e3e41cdb3dad58788d878e31d7d82e8",
33+
"@angular/cli": "^17.0.0",
34+
"@angular/compiler": "17.3.12",
35+
"@angular/compiler-cli": "^17.0.0",
3636
"@babel/core": "^7.16.12",
37-
"@bazel/concatjs": "5.7.0",
38-
"@bazel/esbuild": "5.7.0",
37+
"@bazel/concatjs": "5.8.1",
38+
"@bazel/esbuild": "5.8.1",
3939
"@bazel/ibazel": "^0.15.9",
40-
"@bazel/jasmine": "5.7.0",
41-
"@bazel/terser": "5.7.0",
42-
"@bazel/typescript": "5.7.0",
40+
"@bazel/jasmine": "5.8.1",
41+
"@bazel/terser": "5.8.1",
42+
"@bazel/typescript": "5.8.1",
4343
"@types/d3": "5.7.2",
4444
"@types/jasmine": "^3.8.2",
4545
"@types/lodash": "^4.14.172",
@@ -61,22 +61,22 @@
6161
"prettier-plugin-organize-imports": "2.3.4",
6262
"requirejs": "^2.3.7",
6363
"tslib": "^2.3.0",
64-
"typescript": "4.9.5",
64+
"typescript": "5.2.2",
6565
"yarn-deduplicate": "^5.0.0"
6666
},
6767
"dependencies": {
68-
"@angular/animations": "^16.2.12",
69-
"@angular/cdk": "^16.2.14",
70-
"@angular/common": "16.2.12",
71-
"@angular/core": "^16.2.12",
72-
"@angular/forms": "^16.2.12",
73-
"@angular/localize": "^16.2.12",
74-
"@angular/material": "^16.2.14",
75-
"@angular/platform-browser": "^16.2.12",
76-
"@angular/platform-browser-dynamic": "^16.2.12",
77-
"@angular/router": "^16.2.12",
78-
"@ngrx/effects": "^15.4.0",
79-
"@ngrx/store": "^15.4.0",
68+
"@angular/animations": "^17.0.0",
69+
"@angular/cdk": "^17.0.0",
70+
"@angular/common": "17.3.12",
71+
"@angular/core": "^17.0.0",
72+
"@angular/forms": "^17.0.0",
73+
"@angular/localize": "^17.0.0",
74+
"@angular/material": "^17.0.0",
75+
"@angular/platform-browser": "^17.0.0",
76+
"@angular/platform-browser-dynamic": "^17.0.0",
77+
"@angular/router": "^17.0.0",
78+
"@ngrx/effects": "^17.0.0",
79+
"@ngrx/store": "^17.0.0",
8080
"@polymer/decorators": "^3.0.0",
8181
"@polymer/iron-behaviors": "^3.0.1",
8282
"@polymer/iron-collapse": "^3.0.1",
@@ -131,7 +131,7 @@
131131
"three": "~0.137.0",
132132
"umap-js": "^1.3.2",
133133
"web-animations-js": "^2.3.2",
134-
"zone.js": "^0.13.3"
134+
"zone.js": "^0.14.0"
135135
},
136136
"resolutions": {
137137
"@types/d3-brush": "1.1.8",
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
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
2+
index 870da1b..3f1e5c5 100755
3+
--- a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel
4+
+++ b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel
5+
@@ -23,6 +23,7 @@ js_library(
6+
deps = [
7+
"@npm//@babel/core",
8+
"@npm//@babel/helper-annotate-as-pure",
9+
+ "@npm//@babel/helper-split-export-declaration",
10+
],
11+
)
12+
13+
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
14+
index 6d5ec3f..ad4217f 100755
15+
--- a/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs
16+
+++ b/node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs
17+
@@ -86,11 +86,11 @@ export async function createEsbuildAngularOptimizePlugin(opts, additionalBabelPl
18+
devkitOptimizePlugins.adjustTypeScriptEnumsPlugin,
19+
);
20+
21+
- // If the current file is denoted as explicit side effect free, add the pure
22+
- // top-level functions optimization plugin for this file.
23+
- if (opts.optimize.isSideEffectFree && opts.optimize.isSideEffectFree(args.path)) {
24+
- plugins.push(devkitOptimizePlugins.pureToplevelFunctionsPlugin);
25+
- }
26+
+ // For TensorBoard: This plugin aggressively culls symbols in a way that
27+
+ // is incompatible with TensorBoard source. Disable it. As result the binary is bigger.
28+
+ //if (opts.optimize.isSideEffectFree && opts.optimize.isSideEffectFree(args.path)) {
29+
+ // plugins.push(devkitOptimizePlugins.pureToplevelFunctionsPlugin);
30+
+ //}
31+
}
32+
33+
const shouldRunLinker =

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

Lines changed: 0 additions & 32 deletions
This file was deleted.
Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,17 @@ index fed787a..377915a 100755
2828
return struct(
2929
closure_js = closure_js_files,
3030
devmode_js = devmode_js_files,
31-
diff --git a/node_modules/@bazel/concatjs/web_test/karma.conf.js b/node_modules/@bazel/concatjs/web_test/karma.conf.js
32-
index 90a03ef..28778c9 100755
33-
--- a/node_modules/@bazel/concatjs/web_test/karma.conf.js
34-
+++ b/node_modules/@bazel/concatjs/web_test/karma.conf.js
35-
@@ -384,7 +384,15 @@ try {
36-
conf.browsers.push(launcher);
37-
} else {
38-
const launcher = 'CustomChrome';
39-
- conf.customLaunchers = {[launcher]: {base: browser, flags: additionalArgs}};
40-
+ // For TensorBoard: Patch the CustomChrome launcher so that even it
41-
+ // specifies the --no-sandbox flag. This is to workaround
42-
+ // incompatibilities with some environments.
43-
+ //
44-
+ // Specifically we were seeing errors like:
45-
+ // [WARNING:gpu_process_host.cc(1228)] The GPU process has crashed 6 time(s)
46-
+ // [FATAL:gpu_data_manager_impl_private.cc(439)] GPU process isn't usable. Goodbye.
47-
+ conf.customLaunchers =
48-
+ {[launcher]: {base: browser, flags: ['--no-sandbox', ...additionalArgs]}};
49-
conf.browsers.push(launcher);
50-
}
51-
}
31+
diff --git a/node_modules/@bazel/concatjs/package.json b/node_modules/@bazel/concatjs/package.json
32+
index 1234567..abcdefg 100755
33+
--- a/node_modules/@bazel/concatjs/package.json
34+
+++ b/node_modules/@bazel/concatjs/package.json
35+
@@ -24,7 +24,8 @@
36+
"dependencies": {
37+
"protobufjs": "6.8.8",
38+
"source-map-support": "0.5.9",
39+
- "tsutils": "3.21.0"
40+
+ "tsutils": "3.21.0",
41+
+ "typescript": "5.2.2"
42+
},
43+
"peerDependencies": {
44+
"karma": ">=4.0.0",

patches/README.md

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,39 @@ After creating or updating a patch, ensure there is no trailing whitespace on
77
any line (CI runs `./tensorboard/tools/whitespace_hygiene_test.py`). You can
88
strip it with `sed -i '' 's/[[:space:]]*$//' patches/<patch-file>.patch`.
99

10-
To regenerate @bazel/concatjs patch:
11-
* `vi node_modules/@bazel/concatjs/web_test/karma.conf.js`
10+
## `@bazel+concatjs+5.8.1.patch`
11+
12+
**Modified files:**
13+
- `node_modules/@bazel/concatjs/internal/common/compilation.bzl`
14+
- `node_modules/@bazel/concatjs/package.json`
15+
16+
**What it does:**
17+
Updated patch from 5.7.0 to 5.8.1. This version already includes the TypeScript 5.x fix and Chrome sandbox fix that we had to patch manually in 5.7.0.
18+
Added typescript as a direct dependency because the Bazel sandbox can't find it otherwise.
19+
20+
Why 5.8.1 and not 6.x: rules_nodejs 6.x removed most of the build rules we depend on (concatjs, esbuild, typescript, etc.) and moved them to a separate project (rules_js). This effort will be done in future upgrades.
21+
22+
23+
To regenerate:
24+
* `vi node_modules/@bazel/concatjs/internal/common/compilation.bzl`
25+
* `vi node_modules/@bazel/concatjs/package.json`
1226
* make edits
1327
* `yarn patch-package "@bazel/concatjs"`
1428
* update the WORKSPACE file with the name of the new patch file
1529

1630

17-
To regenerate @angular/build-tooling patch:
31+
## `@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch`
32+
33+
**Modified files:**
34+
- `node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel`
35+
- `node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs`
36+
37+
**What it does:**
38+
Updated for the Angular 17 version of build-tooling, adding the missing Babel dependency and
39+
Disables an optimization plugin that incorrectly removes function calls that Tensorboard depends on runtime.
40+
41+
To regenerate:
42+
* `vi node_modules/@angular/build-tooling/shared-scripts/angular-optimization/BUILD.bazel`
1843
* `vi node_modules/@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs`
1944
* make edits
2045
* `yarn patch-package "@angular/build-tooling"`

tensorboard/defs/defs.bzl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ def tf_ts_library(srcs = [], strict_checks = True, **kwargs):
179179
**kwargs
180180
)
181181

182-
def tf_ng_web_test_suite(name, deps = [], **kwargs):
182+
# The external list is used to exclude node-fetch from the browser bundle since it is not compatible with browsers. This allows us to use tfjs in our web application without running into issues caused by node-fetch. @tensorflow/tfjs-core is used by vz_projector plugin.
183+
def tf_ng_web_test_suite(name, deps = [], external = [], **kwargs):
183184
"""TensorBoard wrapper for the rule for a Karma web test suite.
184185
185186
This uses the Angular team's internal toolchain for bundling
@@ -218,6 +219,7 @@ def tf_ng_web_test_suite(name, deps = [], **kwargs):
218219
workspace_name = "org_tensorflow_tensorboard",
219220
run_angular_linker = False,
220221
platform = "browser",
222+
external = external,
221223
)
222224

223225
karma_web_test_suite(

tensorboard/plugins/projector/vz_projector/BUILD

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,20 @@ tf_ts_library(
140140

141141
tf_ng_web_test_suite(
142142
name = "vz_projector_test",
143+
# This vz_projector plugin depends on @tensorflow/tfjs library.
144+
# tfjs is designed to run in the browser and in Node.js server, so it includes node-fetch for the Node.js side.
145+
# When esbuild bundles everything for the browser, it tries to include node-fetch and fails because it can't resolve Node.js built-ins. The external list just tells esbuild to ignore those server only packages since they'll never be used in the browser.
146+
external = [
147+
"node-fetch",
148+
"stream",
149+
"http",
150+
"https",
151+
"url",
152+
"zlib",
153+
"buffer",
154+
"punycode",
155+
"string_decoder",
156+
],
143157
deps = [
144158
":vz_projector_test_lib",
145159
],

tensorboard/tools/whitespace_hygiene_test.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,14 @@
2525
import sys
2626

2727

28-
exceptions = frozenset([])
28+
# @TODO(@cdavalos7): Remove this exception when the patch file is no longer needed.
29+
# Patch files use a trailing space on blank lines to mark them as context.
30+
# This is required by patch-package and cannot be removed.
31+
exceptions = frozenset(
32+
[
33+
"patches/@angular+build-tooling+0.0.0-2113cd7f66a089ac0208ea84eee672b2529f4f6c.patch",
34+
]
35+
)
2936

3037

3138
@dataclasses.dataclass(frozen=True)

0 commit comments

Comments
 (0)