Skip to content

Use esbuild for javascript bundling. #5829

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Aug 1, 2022
Merged

Use esbuild for javascript bundling. #5829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6c8d8d6
Use esbuild for tb_webapp_binary (prod and dev).
bmd3k Jul 22, 2022
6d1759d
Use esbuild for polymer3_lib_binary (prod and dev).
bmd3k Jul 22, 2022
6e5d9fa
Implement tf_js_binary using esbuild and migrate other usages of
bmd3k Jul 23, 2022
40f602d
Get monaco source editor working by loading require.js after everything
bmd3k Jul 25, 2022
90f6e5d
Closer to done.
bmd3k Jul 28, 2022
c86b42a
Cleanup after corrupt rebase.
bmd3k Jul 28, 2022
b06e4fe
Cleanup for PR.
bmd3k Jul 28, 2022
08fb3fc
Another cleanup pass.
bmd3k Jul 29, 2022
750e402
buildifier.
bmd3k Jul 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions WORKSPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ yarn_install(
yarn_lock = "//:yarn.lock",
)

# Load esbuild rules for bazel.
# https://bazelbuild.github.io/rules_nodejs/esbuild.html
load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")

esbuild_repositories(npm_repository = "npm")

http_archive(
name = "io_bazel_rules_sass",
sha256 = "ee6d527550d42af182673c3718da98bb9205cabdeb08eacc0e3767fa3f2b051a",
Expand Down
7 changes: 1 addition & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@
"@angular/compiler": "^12.2.0",
"@angular/compiler-cli": "^12.2.0",
"@bazel/concatjs": "^4.6.1",
"@bazel/esbuild": "^4.6.2",
"@bazel/ibazel": "^0.15.9",
"@bazel/jasmine": "^4.6.1",
"@bazel/rollup": "^4.6.1",
"@bazel/terser": "^4.6.1",
"@bazel/typescript": "^4.6.1",
"@rollup/plugin-commonjs": "^20.0.0",
"@rollup/plugin-node-resolve": "^13.0.4",
"@types/d3": "5.7.2",
"@types/jasmine": "^3.8.2",
"@types/lodash": "^4.14.172",
Expand All @@ -58,8 +55,6 @@
"prettier": "2.4.1",
"prettier-plugin-organize-imports": "2.3.4",
"requirejs": "^2.3.6",
"rollup": "^2.56.2",
"terser": "^5.14.2",
"tslib": "^2.3.0",
"typescript": "4.5.4",
"yarn-deduplicate": "^5.0.0"
Expand Down
5 changes: 0 additions & 5 deletions tensorboard/defs/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ tb_proto_library(
],
)

exports_files([
"rollup_config.js",
"terser_config.json",
])

ts_library(
name = "strict_types",
srcs = ["strict_type_check.d.ts"],
Expand Down
89 changes: 37 additions & 52 deletions tensorboard/defs/defs.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,11 @@
# limitations under the License.
"""External-only delegates for various BUILD rules."""

load("@npm//@bazel/rollup:index.bzl", "rollup_bundle")
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
Copy link
Contributor

@JamesHollyer JamesHollyer Jul 29, 2022

Choose a reason for hiding this comment

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

Can you move this back down below these other loads for git blame purposes? Or is there a reason this was moved up?

Copy link
Contributor Author

@bmd3k bmd3k Aug 1, 2022

Choose a reason for hiding this comment

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

I moved it up to sort the imports alphabetically.

load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite")
load("@npm//@bazel/esbuild:index.bzl", "esbuild")
load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_library")
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
load("@npm//@bazel/terser:index.bzl", "terser_minified")
load("//tensorboard/defs/internal:js.bzl", _tf_dev_js_binary = "tf_dev_js_binary")

tf_dev_js_binary = _tf_dev_js_binary

def tensorboard_webcomponent_library(**kwargs):
"""Rules referencing this will be deleted from the codebase soon."""
Expand All @@ -29,7 +26,6 @@ def tensorboard_webcomponent_library(**kwargs):
def tf_js_binary(
name,
compile,
deps,
visibility = None,
dev_mode_only = False,
includes_polymer = False,
Expand All @@ -39,61 +35,50 @@ def tf_js_binary(
Args:
name: Name of the target.
compile: whether to compile when bundling. Only used internally.
deps: dependencies of the js_binary.
visibility: visibility of the target.
dev_mode_only: whether the binary is for development. When True, it will
omit the Terser.
omit the minification step.
includes_polymer: whether this binary contains Polymer. Only used
internally.
**kwargs: keyword arguments to rollup_bundle. Please refer to
https://bazelbuild.github.io/rules_nodejs/Built-ins.html#rollup_bundle
**kwargs: Other keyword arguments to esbuild(). Typically used for
entry_point and deps. Please refer to https://esbuild.github.io/api/
for more details.
"""

# `compile` option is used internally but is not used by rollup_bundle.
# Discard it.
internal_rollup_name = name + "_rollup_internal_dbg"
rollup_bundle(
name = internal_rollup_name,
config_file = "//tensorboard/defs:rollup_config.js",
# Must pass `true` here specifically, else the input file argument to
# Rollup (appended by `rollup_binary`) is interpreted as a value for
# the preceding option.
args = ["--failAfterWarnings", "true", "--silent", "true"],
deps = deps + [
"@npm//@rollup/plugin-commonjs",
"@npm//@rollup/plugin-node-resolve",
],
format = "iife",
sourcemap = "false",
visibility = ["//visibility:private"],
**kwargs
)

if dev_mode_only:
internal_result_name = internal_rollup_name
else:
internal_result_name = name + "_terser_internal_min"
terser_minified(
name = internal_result_name,
src = internal_rollup_name,
# Notes about the terser config:
# compress.passes - this is set to '1' to workaround issue with
# terser and threejs. In practice it (surprisingly) generates
# smaller results than when it was previously set to '3'.
config_file = "//tensorboard/defs:terser_config.json",
visibility = ["//visibility:private"],
sourcemap = False,
)

# For some reason, terser_minified is not visible from other targets. Copy
# or re-export seems to work okay.
native.genrule(
# esbuild is a fast JavaScript bundler[1] appropriate for both production
# and development builds.
#
# Bazel documents[2] how to use esbuild bundling with ts_project but we use
# the not-quite-deprecated ts_library rule instead of ts_project. We've
# managed to get esbuild working with ts_library but its long-term support
Copy link
Contributor

@JamesHollyer JamesHollyer Jul 29, 2022

Choose a reason for hiding this comment

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

Did you have to do anything besides specifying .mjs extensions to get ts_library to work?

Copy link
Contributor Author

@bmd3k bmd3k Aug 1, 2022

Choose a reason for hiding this comment

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

That's all I was certain was ts_library-specific.

# is unknown.
#
# [1]: https://esbuild.github.io/
# [2]: https://www.npmjs.com/package/@bazel/esbuild
esbuild(
name = name,
srcs = [internal_result_name],
outs = [name + ".js"],
visibility = visibility,
cmd = "cat $(SRCS) > $@",
# Use "iife" format instead of "esm" because "esm" writes symbols at
# the global level and tends to overwrite `window` functions. "iife" is
# just a thin wrapper around "esm" (it adds 11 bytes) and doesn't
# suffer from the same overwriting problem.
format="iife",
minify= False if dev_mode_only else True,
args = {
# Must specify that 'mjs' extensions are preferred, since that is
# the extension that is used for es2015/esm code generated by
# ts_library.
# https://github.com/bazelbuild/rules_nodejs/issues/2691#issuecomment-846429871
"resolveExtensions": [".mjs", ".js"],
# The reasoning for these particular mainFields values are lost to
# history. These come from the old rollup bundler configuration.
# We do know that the esbuild default values for mainFields do not
# work for us. In particular we ran into problems with
# esbuild pulling in "node"-specific versions of some libraries that
# are incompatible with browsers.
"mainFields": ["browser", "es2015", "module", "jsnext:main", "main"],
},
**kwargs
)

def tf_ts_config(**kwargs):
Expand Down
149 changes: 0 additions & 149 deletions tensorboard/defs/internal/js.bzl
View file
Open in desktop

This file was deleted.

56 changes: 0 additions & 56 deletions tensorboard/defs/rollup_config.js
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions tensorboard/defs/terser_config.json
View file
Open in desktop

This file was deleted.

Loading