Migrate to Angular's app_bundle for bundling. #6049
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
JamesHollyer
left a comment
JamesHollyer
left a comment
There was a problem hiding this comment.
So no comments on the code change but, I would like some clarification.
The description confused me. It states "This change uses Angular's internal app_bundle() macro to allow us to continue to run Karma tests with Angular 13+." This is used in development and production builds right? I thought spec_bundle fixed the tests.
Also, is this bundling necessary to run Angular 13? I thought the plan was to just use esbuild in development builds to cut down on that increased bundling time from app_bundle. Then we just use app_bundle for production builds. Is it needed because it does that new Angular "linking" stage?
Contributor
Author
Sorry, that's a copy and paste error. Hopefully it makes sense now.
Wow I am glad you remember that because I clearly forgot. Using app_bundle() only for prod Angular bundle is a much nicer conclusion. I've updated the PR code and description. This should be fine with the Angular 13 upgrade as we can have "//tensorboard:dev" use Angular JIT compilation without any trouble or noticeable impact on app load times in the browser. |
JamesHollyer
approved these changes
Nov 17, 2022
Contributor
JamesHollyer
left a comment
JamesHollyer
left a comment
There was a problem hiding this comment.
"No impact on either bundle size or bundle time" Woohoo!
Glad I could help. LGTM.
bmd3k
added a commit
that referenced
this pull request
Nov 22, 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: * #6036 * #6049 * Some tests were previously cleaned up in preparation for this change: * #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.
qihach64
pushed a commit
to qihach64/tensorboard
that referenced
this pull request
Dec 19, 2022
The upcoming upgrade to Angular 13+ introduces a number of bundling-related problems for our tests and applications. Angular has developed some internal tooling to help resolve these problems. This change uses Angular's internal app_bundle() macro to allow us to continue to bundle Angular13+-compatible apps. Note: app_bundle() is Angular-internal and not supported for external use. We use it at our own risk. Googlers, see the following documents for background and justification: http://go/angular-bazel-problems, http://go/tb-oss-bundling. Note: We migrated our tests to use the similar spec_bundle() in tensorflow#6036. Our usage of app_bundle() is largely guided/inspired by the usage in the angular/components repo. This is the version at HEAD as of early November: https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl We will now have two bundling targets: tf_js_binary(), which remains unchanged; and tf_ng_prod_js_binary(), which is new and delegates to app_bundle(). tf_ng_prod_js_binary() is, in practice, only used for one bundle: The production Angular bundle at //tensorboard/webapp:tb_webapp_binary. All other bundles, including our dev Angular bundle, continue to use tf_js_binary(). Googlers, see detailed reasoning in http://go/tb-oss-bundling. Highlights. 1. Run the following commands to install additional Angular build tooling and terser and then clean the environment: * `yarn add @angular-devkit/build-angular@14.0.0-next.3 @bazel/terser@5.7.0 --dev` * `yarn run yarn-deduplicate` * `rm -rf node_modules; bazel clean --expunge; yarn;` 2. Patch the Angular build tooling to suit our code base: * Patch the esbuild config to point to old paths for tools in @angular/compiler-cli. When we upgrade @angular/compiler-cli to Angular 13+ then we will be able to delete this portion of the patch. * Patch some of the babel-based optimization code to remove one particular plugin that is too aggressive in removing symbols. It is incompatible with the TensorBoard code base. 3. Add tf_ng_prod_js_binary, which delegates to app_bundle(), and use it for //tensorboard/webapp:tb_webapp_binary. Impact on "//tensorboard" build: * "//tensorboard/webapp:tb_webapp_binary" bundle size: * Before: 5,440,612 bytes * After: 4,790,003 * "//tensorboard/webapp:tb_webapp_binary" bundle time: * Before: 1 or 2 seconds. * After: ~20 seconds. * This is an ok tradeoff since we continue to encourage engineers to use the faster "//tensorboard:dev" and its dev bundle for local development. Impact on "//tensorboard:dev" target: * No impact on either bundle size or bundle time since we continue to use tf_js_binary(). It remains big but fast. * Note that when we update to Angular 13 we will have to change "//tensorboard:dev" to use Angular JIT compilation. Initial observations show that the JIT compilation does not meaningfully slow down app load or interaction times.
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
The upcoming upgrade to Angular 13+ introduces a number of bundling-related problems for our tests and applications. Angular has developed some internal tooling to help resolve these problems. This change uses Angular's internal app_bundle() macro to allow us to continue to bundle Angular13+-compatible apps. Note: app_bundle() is Angular-internal and not supported for external use. We use it at our own risk. Googlers, see the following documents for background and justification: http://go/angular-bazel-problems, http://go/tb-oss-bundling. Note: We migrated our tests to use the similar spec_bundle() in tensorflow#6036. Our usage of app_bundle() is largely guided/inspired by the usage in the angular/components repo. This is the version at HEAD as of early November: https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl We will now have two bundling targets: tf_js_binary(), which remains unchanged; and tf_ng_prod_js_binary(), which is new and delegates to app_bundle(). tf_ng_prod_js_binary() is, in practice, only used for one bundle: The production Angular bundle at //tensorboard/webapp:tb_webapp_binary. All other bundles, including our dev Angular bundle, continue to use tf_js_binary(). Googlers, see detailed reasoning in http://go/tb-oss-bundling. Highlights. 1. Run the following commands to install additional Angular build tooling and terser and then clean the environment: * `yarn add @angular-devkit/build-angular@14.0.0-next.3 @bazel/terser@5.7.0 --dev` * `yarn run yarn-deduplicate` * `rm -rf node_modules; bazel clean --expunge; yarn;` 2. Patch the Angular build tooling to suit our code base: * Patch the esbuild config to point to old paths for tools in @angular/compiler-cli. When we upgrade @angular/compiler-cli to Angular 13+ then we will be able to delete this portion of the patch. * Patch some of the babel-based optimization code to remove one particular plugin that is too aggressive in removing symbols. It is incompatible with the TensorBoard code base. 3. Add tf_ng_prod_js_binary, which delegates to app_bundle(), and use it for //tensorboard/webapp:tb_webapp_binary. Impact on "//tensorboard" build: * "//tensorboard/webapp:tb_webapp_binary" bundle size: * Before: 5,440,612 bytes * After: 4,790,003 * "//tensorboard/webapp:tb_webapp_binary" bundle time: * Before: 1 or 2 seconds. * After: ~20 seconds. * This is an ok tradeoff since we continue to encourage engineers to use the faster "//tensorboard:dev" and its dev bundle for local development. Impact on "//tensorboard:dev" target: * No impact on either bundle size or bundle time since we continue to use tf_js_binary(). It remains big but fast. * Note that when we update to Angular 13 we will have to change "//tensorboard:dev" to use Angular JIT compilation. Initial observations show that the JIT compilation does not meaningfully slow down app load or interaction times.
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
The upcoming upgrade to Angular 13+ introduces a number of bundling-related problems for our tests and applications. Angular has developed some internal tooling to help resolve these problems. This change uses Angular's internal app_bundle() macro to allow us to continue to bundle Angular13+-compatible apps. Note: app_bundle() is Angular-internal and not supported for external use. We use it at our own risk. Googlers, see the following documents for background and justification: http://go/angular-bazel-problems, http://go/tb-oss-bundling. Note: We migrated our tests to use the similar spec_bundle() in tensorflow#6036. Our usage of app_bundle() is largely guided/inspired by the usage in the angular/components repo. This is the version at HEAD as of early November: https://github.com/angular/components/blob/871f8f231a7a86a7a0778e345f4d517109c9a357/tools/defaults.bzl We will now have two bundling targets: tf_js_binary(), which remains unchanged; and tf_ng_prod_js_binary(), which is new and delegates to app_bundle(). tf_ng_prod_js_binary() is, in practice, only used for one bundle: The production Angular bundle at //tensorboard/webapp:tb_webapp_binary. All other bundles, including our dev Angular bundle, continue to use tf_js_binary(). Googlers, see detailed reasoning in http://go/tb-oss-bundling. Highlights. 1. Run the following commands to install additional Angular build tooling and terser and then clean the environment: * `yarn add @angular-devkit/build-angular@14.0.0-next.3 @bazel/terser@5.7.0 --dev` * `yarn run yarn-deduplicate` * `rm -rf node_modules; bazel clean --expunge; yarn;` 2. Patch the Angular build tooling to suit our code base: * Patch the esbuild config to point to old paths for tools in @angular/compiler-cli. When we upgrade @angular/compiler-cli to Angular 13+ then we will be able to delete this portion of the patch. * Patch some of the babel-based optimization code to remove one particular plugin that is too aggressive in removing symbols. It is incompatible with the TensorBoard code base. 3. Add tf_ng_prod_js_binary, which delegates to app_bundle(), and use it for //tensorboard/webapp:tb_webapp_binary. Impact on "//tensorboard" build: * "//tensorboard/webapp:tb_webapp_binary" bundle size: * Before: 5,440,612 bytes * After: 4,790,003 * "//tensorboard/webapp:tb_webapp_binary" bundle time: * Before: 1 or 2 seconds. * After: ~20 seconds. * This is an ok tradeoff since we continue to encourage engineers to use the faster "//tensorboard:dev" and its dev bundle for local development. Impact on "//tensorboard:dev" target: * No impact on either bundle size or bundle time since we continue to use tf_js_binary(). It remains big but fast. * Note that when we update to Angular 13 we will have to change "//tensorboard:dev" to use Angular JIT compilation. Initial observations show that the JIT compilation does not meaningfully slow down app load or interaction times.
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 openxla/xprof
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The upcoming upgrade to Angular 13+ introduces a number of bundling-related problems for our tests and applications. Angular has developed some internal tooling to help resolve these problems. This change uses Angular's internal app_bundle() macro to allow us to continue to bundle Angular13+-compatible apps.
Note: app_bundle() is Angular-internal and not supported for external use. We use it at our own risk. Googlers, see the following documents for background and justification: http://go/angular-bazel-problems, http://go/tb-oss-bundling.
Note: We migrated our tests to use the similar spec_bundle() in #6036.
Our usage of app_bundle() is largely guided/inspired by the usage in the angular/components repo. This is the version at HEAD as of early November:
We will now have two bundling targets: tf_js_binary(), which remains unchanged; and tf_ng_prod_js_binary(), which is new and delegates to app_bundle(). tf_ng_prod_js_binary() is, in practice, only used for one bundle: The production Angular bundle at //tensorboard/webapp:tb_webapp_binary. All other bundles, including our dev Angular bundle, continue to use tf_js_binary(). Googlers, see detailed reasoning in http://go/tb-oss-bundling.
Highlights.
yarn add @angular-devkit/build-angular@14.0.0-next.3 @bazel/terser@5.7.0 --devyarn run yarn-deduplicaterm -rf node_modules; bazel clean --expunge; yarn;Impact on "//tensorboard" build:
Impact on "//tensorboard:dev" target: