feat: make esbuild_bundle path-mapping-friendly - #289
Merged
Conversation
✨ Aspect Workflows Tasks📅 Fri Aug 7 07:03:52 UTC 2026 ✅ 28 successful tasks
⏱ Last updated Fri Aug 7 07:06:03 UTC 2026 · 📊 GitHub API quota 5,378/15,000 (36% used, resets in 51m, throttle 4×) |
acozzette
marked this pull request as ready for review
August 5, 2026 23:55
jbedard
reviewed
Aug 6, 2026
jbedard
reviewed
Aug 6, 2026
acozzette
commented
Aug 6, 2026
BAZEL_BINDIR was baked into the esbuild action's env dict as a literal string, which Bazel's path-mapping feature never rewrites, defeating any cross-compilation-mode cache sharing for esbuild actions. esbuild_bundle now passes BAZEL_BINDIR to its launcher via aspect_rules_js's js_binary_lib.run_binary_action() helper, which conveys it through a path-mapping-friendly command-line flag instead. That helper only works when the launcher is actually a js_binary, so a small aspect (launcher_kind_aspect in esbuild/private/helpers.bzl) is attached to the launcher attribute of both esbuild_toolchain() and esbuild_bundle() to detect this automatically by checking the target's underlying rule kind -- no explicit signal needed from toolchain authors, and it works through macros wrapping js_binary and alias() targets. When the launcher isn't a js_binary (see e2e/toolchain_from_source, which uses a plain sh_binary), esbuild_bundle falls back to a plain, non-path-mapped BAZEL_BINDIR environment variable and doesn't advertise supports-path-mapping. Also fixes the bazel-sandbox esbuild plugin, which used to string-match resolved paths against the literal BAZEL_BINDIR value -- under path mapping that value is a generic placeholder while symlink-resolved real paths always contain the true per-config bindir, so it now matches by bindir prefix instead. Adds an e2e/path_mapping test (ported from rules_webpack) that proves cache sharing across -c fastbuild/-c opt via a shared disk cache and checks that supports-path-mapping is advertised/not-advertised correctly. Temporarily pins aspect_rules_js via git_override to a commit containing js_binary_lib.run_binary_action(), since that helper hasn't shipped in a release yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jbedard
reviewed
Aug 6, 2026
jbedard
reviewed
Aug 6, 2026
jbedard
reviewed
Aug 6, 2026
jbedard
reviewed
Aug 6, 2026
Member
|
LGTM, just need the rules_js release 👍 |
jbedard
approved these changes
Aug 7, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
BAZEL_BINDIR was baked into the esbuild action's env dict as a literal string, which Bazel's path-mapping feature never rewrites, defeating any cross-compilation-mode cache sharing for esbuild actions.
esbuild_bundle now passes BAZEL_BINDIR to its launcher via aspect_rules_js's js_binary_lib.run_binary_action() helper, which conveys it through a path-mapping-friendly command-line flag instead. That helper only works when the launcher is actually a js_binary, so a small aspect (launcher_kind_aspect in esbuild/private/helpers.bzl) is attached to the launcher to detect this automatically by checking the target's underlying rule kind. When the launcher isn't a js_binary (see e2e/toolchain_from_source, which uses a plain sh_binary), esbuild_bundle falls back to a plain, non-path-mapped BAZEL_BINDIR environment variable and doesn't advertise supports-path-mapping.
Also fixes the bazel-sandbox esbuild plugin, which used to string-match resolved paths against the literal BAZEL_BINDIR value -- under path mapping that value is a generic placeholder while symlink-resolved real paths always contain the true per-config bindir, so it now matches by bindir prefix instead.
Adds an e2e/path_mapping test that proves cache sharing across -c fastbuild/-c opt via a shared disk cache and checks that supports-path-mapping is advertised/not-advertised correctly.
Changes are visible to end-users: yes
Make esbuild_bundle support path mapping
Test plan