Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Upgrade Angular (to 13) and related dependencies #6063

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

bmd3k
Copy link
Contributor

@bmd3k bmd3k commented Nov 21, 2022

Upgrade Angular to version 13 and related dependencies appropriately.

See: https://update.angular.io/?l=3&v=12.0-13.0
See: https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824

Highlights:

  • Upgrade the following set of dependencies:
    • @angular/*: 13.3.X
    • @ngrx/*: 13.2.X
    • rxjs: 7.4.X
    • zone.js 0.11.6 or higher
    • ngx-color-picker: 12.0.1
  • Some build tooling was previously upgraded in preparation for this change:
  • Some tests were previously cleaned up in preparation for this change:
  • No longer need ngcc as a post install step. All Angular-related dependencies come with partially-compiled binaries.
  • The tensorboard:dev Angular binary now includes the runtime JIT compiler and no longer relies on AOT compilation. Googlers, see http://go/tb-oss-bundling for justification.
  • We can get rid of some of the patches to Angular build tooling that we needed while we used an old version of @angular/compiler-cli.
  • Impact on binaries and performance:
    • There isn't much impact on production binary sizes. The '//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467 bytes to 2,559,890 bytes.
    • The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from 6,074,186 bytes to 7,005,529, likely because it now includes the Angular compiler. It now relies on JIT compilation but there does not seem to be noticeable impact on runtime performance. The development experience is still relatively smooth.

@bmd3k bmd3k requested a review from rileyajones November 22, 2022 12:24
Copy link
Contributor

@rileyajones rileyajones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for doing all this, it's been quite a journey.
I clean compiled this both locally and using Docker and got no issues. I then clicked through all the dashboards and didn't see anything visibly off.

@bmd3k
Copy link
Contributor Author

bmd3k commented Nov 22, 2022

LGTM, thanks for doing all this, it's been quite a journey. I clean compiled this both locally and using Docker and got no issues. I then clicked through all the dashboards and didn't see anything visibly off.

Thanks! The journey is not done yet, though. Angular 14 upgrade coming up.

@bmd3k bmd3k merged commit 471f1f2 into tensorflow:master Nov 22, 2022
bmd3k added a commit to bmd3k/tensorboard that referenced this pull request Nov 23, 2022
…orflow#6063)"

This reverts commit 471f1f2. It breaks
the sync with the internal repo.
qihach64 pushed a commit to qihach64/tensorboard that referenced this pull request Dec 19, 2022
)

Upgrade Angular to version 13 and related dependencies appropriately.

See: https://update.angular.io/?l=3&v=12.0-13.0
See:
https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824

Highlights:

* Upgrade the following set of dependencies:
  * `@angular/*`: 13.3.X
  * `@ngrx/*`: 13.2.X
  * `rxjs`: 7.4.X
  * `zone.js` 0.11.6 or higher
  * `ngx-color-picker`: 12.0.1

* Some build tooling was previously upgraded in preparation for this
change:
  * tensorflow#6036
  * tensorflow#6049

* Some tests were previously cleaned up in preparation for this change:
  * tensorflow#6056 

* No longer need ngcc as a post install step. All Angular-related
dependencies come with partially-compiled binaries.

* The tensorboard:dev Angular binary now includes the runtime JIT
compiler and no longer relies on AOT compilation. Googlers, see
http://go/tb-oss-bundling for justification.

* We can get rid of some of the patches to Angular build tooling that we
needed while we used an old version of @angular/compiler-cli.

* Impact on binaries and performance:
  * There isn't much impact on production binary sizes. The
'//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467
bytes to 2,559,890 bytes.
  * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from
6,074,186 bytes to 7,005,529, likely because it now includes the Angular
compiler. It now relies on JIT compilation but there does not seem to be
noticeable impact on runtime performance. The development experience is
still relatively smooth.
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
)

Upgrade Angular to version 13 and related dependencies appropriately.

See: https://update.angular.io/?l=3&v=12.0-13.0
See:
https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824

Highlights:

* Upgrade the following set of dependencies:
  * `@angular/*`: 13.3.X
  * `@ngrx/*`: 13.2.X
  * `rxjs`: 7.4.X
  * `zone.js` 0.11.6 or higher
  * `ngx-color-picker`: 12.0.1

* Some build tooling was previously upgraded in preparation for this
change:
  * tensorflow#6036
  * tensorflow#6049

* Some tests were previously cleaned up in preparation for this change:
  * tensorflow#6056 

* No longer need ngcc as a post install step. All Angular-related
dependencies come with partially-compiled binaries.

* The tensorboard:dev Angular binary now includes the runtime JIT
compiler and no longer relies on AOT compilation. Googlers, see
http://go/tb-oss-bundling for justification.

* We can get rid of some of the patches to Angular build tooling that we
needed while we used an old version of @angular/compiler-cli.

* Impact on binaries and performance:
  * There isn't much impact on production binary sizes. The
'//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467
bytes to 2,559,890 bytes.
  * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from
6,074,186 bytes to 7,005,529, likely because it now includes the Angular
compiler. It now relies on JIT compilation but there does not seem to be
noticeable impact on runtime performance. The development experience is
still relatively smooth.
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
)

Upgrade Angular to version 13 and related dependencies appropriately.

See: https://update.angular.io/?l=3&v=12.0-13.0
See:
https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824

Highlights:

* Upgrade the following set of dependencies:
  * `@angular/*`: 13.3.X
  * `@ngrx/*`: 13.2.X
  * `rxjs`: 7.4.X
  * `zone.js` 0.11.6 or higher
  * `ngx-color-picker`: 12.0.1

* Some build tooling was previously upgraded in preparation for this
change:
  * tensorflow#6036
  * tensorflow#6049

* Some tests were previously cleaned up in preparation for this change:
  * tensorflow#6056 

* No longer need ngcc as a post install step. All Angular-related
dependencies come with partially-compiled binaries.

* The tensorboard:dev Angular binary now includes the runtime JIT
compiler and no longer relies on AOT compilation. Googlers, see
http://go/tb-oss-bundling for justification.

* We can get rid of some of the patches to Angular build tooling that we
needed while we used an old version of @angular/compiler-cli.

* Impact on binaries and performance:
  * There isn't much impact on production binary sizes. The
'//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467
bytes to 2,559,890 bytes.
  * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from
6,074,186 bytes to 7,005,529, likely because it now includes the Angular
compiler. It now relies on JIT compilation but there does not seem to be
noticeable impact on runtime performance. The development experience is
still relatively smooth.
copybara-service bot pushed a commit to tensorflow/profiler that referenced this pull request Jun 5, 2023
Few changes:
- upgrade rules_nodejs to 5.7.0 (this will extend the nodejs versions available for installation, while we are installing the default nodejs version here with the 5.7.0 rules_nodejs toolchain)
  - following this, upgrade all bazel toolchain to 5..7.0
- correspondingly upgrade rules_sass version to 1.55.0 (I can only find rules_sass github codebase with given version tag, but not sure how to find its hash..etc. Here we are basically following tensorboard team's current setup)
- upgrade angular (and all relative packages) to v14, correspondingly upgrade ngrx to v14, and bump up rxjs.
- upgrade typescript to 4.7.4
- upgrade zone.js to 0.12.0
- import ts_library (used for bazel build) from concatjs instead of typescript, as the module is moved to concatjs. (Also added concatjs into data field of `tsc_wrapped_with_angular` target)
- added name field to the output of `rollup.config.js`, as it is a required field now for iife output type
- Added `node_modules` into `include_paths` of sass_binary build rule when building the `styles.css` file for frontend. This is because angular v14 changed to import angular/cdk from using a relative path to an new one like `@angular/cdk`, so we need to include the `node_modules` int the the searching paths to enable sassCompiler to find the theming file.
- yarn_install's exports_directories_only property now defaults to True. We must set this to False in order to remain compatible with ts_library
See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#exports_directories_only
- No longer need to override node_version as rules_nodejs default version of node is now 16.12.0.
See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#updated-defaults
We were overriding this only because rules_nodejs defaul version of node was too old.

**References**
- migrating to rules_nodejs 5.0: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0
  - Instructions for 5.7.0 specifically: https://github.com/bazelbuild/rules_nodejs/releases/tag/5.7.0
- Tensorboard team's angular upgrade notes: No longer need to override node_version as rules_nodejs default version of node is now 16.12.0.
See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#updated-defaults
We were overriding this only because rules_nodejs defaul version of node was too old.
- other relative tensorboard upgrade PRs (tons of information in the PR messasge):
tensorflow/tensorboard#5977
tensorflow/tensorboard#6063
tensorflow/tensorboard#6066

**Notes**
Since then, the tensorboard_plugin_profiler build process mostly follows tensorboard's setup, but we are diverging moving forward as following:
(1) The tensorboard team changed to use esbuild to bundle the js file instead of rollup (see tensorflow/tensorboard#5829). We may consider switching to esbuild in the future as well given its potential better performance, but now since it's not blocking us, we remain to use rollup.
(2) The tensorboard team uses angular teams internal toolchain to build angular with bazel for angular v13+ (see tensorflow/tensorboard#6049). Since we turns out don't have the issue tensorboard is facing when upgrading angular, we are fine to go forward without changing our bundling rules. And we tend not to because the toolchain is not publicly used and there's no guarantee on supporting in the future.

PiperOrigin-RevId: 537957341
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants