Skip to content

Commit

Permalink
Migrate to Angular's app_bundle for bundling. (tensorflow#6049)
Browse files Browse the repository at this point in the history
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/[email protected]
@bazel/[email protected] --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.
  • Loading branch information
bmd3k authored Nov 21, 2022
1 parent ed5e9d0 commit 47c1b57
Show file tree
Hide file tree
Showing 6 changed files with 1,581 additions and 152 deletions.
5 changes: 4 additions & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

yarn_install(
name = "npm",
data = ["//patches:@bazel+concatjs+5.7.0.patch"],
data = [
"//patches:@angular+build-tooling+0.0.0-7d103b83a07f132629592fc9918ce17d42a5e382.patch",
"//patches:@bazel+concatjs+5.7.0.patch",
],
# "Some rules only work by referencing labels nested inside npm packages
# and therefore require turning off exports_directories_only."
# This includes "ts_library".
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
},
"homepage": "https://github.com/tensorflow/tensorboard#readme",
"devDependencies": {
"@angular-devkit/build-angular": "14.0.0-next.3",
"@angular/build-tooling": "https://github.com/angular/dev-infra-private-build-tooling-builds.git#fb42478534df7d48ec23a6834fea94a776cb89a0",
"@angular/cli": "^12.2.0",
"@angular/compiler": "^12.2.0",
Expand All @@ -37,6 +38,7 @@
"@bazel/esbuild": "5.7.0",
"@bazel/ibazel": "^0.15.9",
"@bazel/jasmine": "5.7.0",
"@bazel/terser": "5.7.0",
"@bazel/typescript": "5.7.0",
"@types/d3": "5.7.2",
"@types/jasmine": "^3.8.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
diff --git a/node_modules/@angular/build-tooling/bazel/app-bundling/esbuild.config-tmpl.mjs b/node_modules/@angular/build-tooling/bazel/app-bundling/esbuild.config-tmpl.mjs
index 618bbc5..05a112f 100755
--- a/node_modules/@angular/build-tooling/bazel/app-bundling/esbuild.config-tmpl.mjs
+++ b/node_modules/@angular/build-tooling/bazel/app-bundling/esbuild.config-tmpl.mjs
@@ -9,9 +9,18 @@
import * as path from 'path';

import {createEsbuildAngularOptimizePlugin} from '@angular/build-tooling/shared-scripts/angular-optimization/esbuild-plugin.mjs';
-import {createEs2015LinkerPlugin} from '@angular/compiler-cli/linker/babel';
-import {ConsoleLogger, NodeJSFileSystem, LogLevel} from '@angular/compiler-cli';
-import {GLOBAL_DEFS_FOR_TERSER_WITH_AOT} from '@angular/compiler-cli/private/tooling';
+// For TensorBoard: Patch the esbuild config template so that imports are
+// grabbed from the correct places. This is just the consequence of some
+// skew using pre-13.X @angular/compiler-cli binaries. This should be resolved
+// once we upgrade to Angular 13+.
+//import {createEs2015LinkerPlugin} from '@angular/compiler-cli/linker/babel';
+import {createEs2015LinkerPlugin} from '@angular/compiler-cli/linker/babel/index.js';
+//import {ConsoleLogger, NodeJSFileSystem, LogLevel} from '@angular/compiler-cli';
+import {NodeJSFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.js';
+import {LogLevel} from '@angular/compiler-cli/src/ngtsc/logging/index.js';
+import {ConsoleLogger} from '@angular/compiler-cli/src/ngtsc/logging/src/console_logger.js';
+//import {GLOBAL_DEFS_FOR_TERSER_WITH_AOT} from '@angular/compiler-cli/private/tooling';
+import {GLOBAL_DEFS_FOR_TERSER_WITH_AOT} from '@angular/compiler-cli/src/tooling.js';

/** Root path pointing to the app bundle source entry-point file. */
const entryPointSourceRootPath = path.normalize(`TMPL_ENTRY_POINT_ROOTPATH`);
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
--- 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,
52 changes: 48 additions & 4 deletions tensorboard/defs/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
load("@npm//@angular/build-tooling/bazel/app-bundling:index.bzl", "app_bundle")
load("@npm//@angular/build-tooling/bazel/spec-bundling:index.bzl", "spec_bundle")
load("@npm//@angular/build-tooling/bazel:extract_js_module_output.bzl", "extract_js_module_output")
load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite", "ts_library")
Expand All @@ -33,7 +34,13 @@ def tf_js_binary(
dev_mode_only = False,
includes_polymer = False,
**kwargs):
"""Rules for creating a JavaScript bundle.
"""Rule for creating a JavaScript bundle.
This uses esbuild() directly and is generally used for any bundle that is
non-Angular or non-Prod. It is faster than tf_ng_prod_js_binary.
Angular apps that use this rule will have to be run with the Angular JIT
compiler as this rule does not support Angular AOT compilation.
Args:
name: Name of the target.
Expand Down Expand Up @@ -84,6 +91,43 @@ def tf_js_binary(
**kwargs
)


def tf_ng_prod_js_binary(
name,
compile,
**kwargs):
"""Rule for creating a prod-optimized JavaScript bundle for an Angular app.
This uses the Angular team's internal toolchain for creating these bundles:
app_bundle(). This toolchain is not officially supported. We use it at our
own risk.
The bundles allow for Angular AOT compilation and are further optimized to
reduce size. However, the bundle times are significantly slower than those
for tf_js_binary().
Args:
name: Name of the target.
compile: Whether to compile when bundling. Only used internally.
**kwargs: Other keyword arguments to app_bundle() and esbuild(). Typically
used for entry_point and deps. Please refer to
https://esbuild.github.io/api/ for more details.
"""

app_bundle_name = '%s_app_bundle' % name
app_bundle(
name = app_bundle_name,
**kwargs
)

# app_bundle() generates several outputs. We copy the one that has gone
# through a terser pass to be the output of this rule.
copy_file(
name = name,
src = '%s.min.js' % app_bundle_name,
out = '%s.js' % name,
)

def tf_ts_config(**kwargs):
"""TensorBoard wrapper for the rule for a TypeScript configuration."""

Expand Down Expand Up @@ -137,9 +181,9 @@ def tf_ts_library(srcs = [], strict_checks = True, **kwargs):
def tf_ng_web_test_suite(name, deps = [], **kwargs):
"""TensorBoard wrapper for the rule for a Karma web test suite.
We use the Angular team's internal toolchain for bundling Angular-compatible
tests: extract_js_module_output() and spec_bundle(). This toolchain is not
officially supported and is subject to change or deletion.
This uses the Angular team's internal toolchain for bundling
Angular-compatible tests: extract_js_module_output() and spec_bundle().
This toolchain is not officially supported. We use it at our own risk.
"""

# Call extract_js_module_output() to prepare proper input for spec_bundle()
Expand Down
4 changes: 2 additions & 2 deletions tensorboard/webapp/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("//tensorboard/defs:defs.bzl", "tf_external_sass_libray", "tf_js_binary", "tf_ng_module", "tf_ng_web_test_suite", "tf_sass_binary", "tf_svg_bundle", "tf_ts_library")
load("//tensorboard/defs:defs.bzl", "tf_external_sass_libray", "tf_ng_module", "tf_ng_prod_js_binary", "tf_ng_web_test_suite", "tf_sass_binary", "tf_svg_bundle", "tf_ts_library")
load("//tensorboard/defs:js.bzl", "tf_resource_digest_suffixer")
load("//tensorboard/defs:web.bzl", "tb_combine_html", "tf_web_library")

Expand Down Expand Up @@ -173,7 +173,7 @@ tf_ng_module(
)

# Compile the prepared Angular app to a JS binary.
tf_js_binary(
tf_ng_prod_js_binary(
name = "tb_webapp_binary",
compile = 1,
entry_point = "main_prod.ts",
Expand Down
Loading

0 comments on commit 47c1b57

Please sign in to comment.