From 6161993c0c2799bb0c52dfa73a22925eacc62b2c Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Mon, 10 Nov 2025 10:30:33 +1100 Subject: [PATCH] [bazel] Switch to custom `closure_js_deps` rule After `rules_closure` 0.12.0, the `closure_js_deps` rule was removed and it was advised to use an `npm` invocation to generate the deps file. However, we still want to use the deps file as it makes it a lot easier to iterate on the atoms when we work on them. The obvious solution is to write our own implementation, which is what happens in this commit. --- .bazelignore | 1 + MODULE.bazel | 1 + javascript/atoms/BUILD.bazel | 4 +- javascript/atoms/package.json | 14 +++ javascript/chrome-driver/BUILD.bazel | 4 +- javascript/defs.bzl | 2 + javascript/ie-driver/BUILD.bazel | 4 +- javascript/private/BUILD.bazel | 6 + javascript/private/closure_js_deps.bzl | 117 ++++++++++++++++++ javascript/webdriver/BUILD.bazel | 4 +- javascript/webdriver/atoms/inject/BUILD.bazel | 4 +- pnpm-lock.yaml | 14 +++ pnpm-workspace.yaml | 1 + 13 files changed, 166 insertions(+), 10 deletions(-) create mode 100644 javascript/atoms/package.json create mode 100644 javascript/private/closure_js_deps.bzl diff --git a/.bazelignore b/.bazelignore index 8b40e6384ed79..a4da41747e7da 100644 --- a/.bazelignore +++ b/.bazelignore @@ -22,6 +22,7 @@ dotnet/src/webdriver/obj java/build/production java/client/build java/server/build +javascript/atoms/node_modules javascript/grid-ui/node_modules javascript/selenium-webdriver/node_modules node_modules diff --git a/MODULE.bazel b/MODULE.bazel index 023b5a236026e..e61d2d08dca31 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -66,6 +66,7 @@ npm.npm_translate_lock( data = [ "@//:package.json", "@//:pnpm-workspace.yaml", + "@//javascript/atoms:package.json", "@//javascript/grid-ui:package.json", "@//javascript/selenium-webdriver:package.json", ], diff --git a/javascript/atoms/BUILD.bazel b/javascript/atoms/BUILD.bazel index d73b9e7abd22b..02b961abf2fd7 100644 --- a/javascript/atoms/BUILD.bazel +++ b/javascript/atoms/BUILD.bazel @@ -1,5 +1,5 @@ -load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_deps", "closure_js_library") -load("//javascript:defs.bzl", "closure_test_suite") +load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") +load("//javascript:defs.bzl", "closure_js_deps", "closure_test_suite") package(default_visibility = ["//visibility:public"]) diff --git a/javascript/atoms/package.json b/javascript/atoms/package.json new file mode 100644 index 0000000000000..b7f7d7dcbea01 --- /dev/null +++ b/javascript/atoms/package.json @@ -0,0 +1,14 @@ +{ + "name": "selenium-atoms", + "version": "1.0.0", + "private": true, + "description": "Build tools for Selenium Browser Automation Atoms", + "license": "Apache-2.0", + "repository": { + "type": "git", + "url": "https://github.com/SeleniumHQ/selenium.git" + }, + "devDependencies": { + "google-closure-deps": "^20230802.0.0" + } +} diff --git a/javascript/chrome-driver/BUILD.bazel b/javascript/chrome-driver/BUILD.bazel index dfccf371882dc..f06b66c43718b 100644 --- a/javascript/chrome-driver/BUILD.bazel +++ b/javascript/chrome-driver/BUILD.bazel @@ -1,5 +1,5 @@ -load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_deps", "closure_js_library") -load("//javascript:defs.bzl", "closure_fragment", "closure_lang_file", "closure_test_suite") +load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") +load("//javascript:defs.bzl", "closure_fragment", "closure_js_deps", "closure_lang_file", "closure_test_suite") closure_js_library( name = "lib", diff --git a/javascript/defs.bzl b/javascript/defs.bzl index 7b583e49b2f85..d8a73c2435dd6 100644 --- a/javascript/defs.bzl +++ b/javascript/defs.bzl @@ -1,9 +1,11 @@ +load("//javascript/private:closure_js_deps.bzl", _closure_js_deps = "closure_js_deps") load("//javascript/private:fragment.bzl", _closure_fragment = "closure_fragment") load("//javascript/private:header.bzl", _closure_lang_file = "closure_lang_file") load("//javascript/private:mocha_test.bzl", _mocha_test = "mocha_test") load("//javascript/private:test_suite.bzl", _closure_test_suite = "closure_test_suite") closure_fragment = _closure_fragment +closure_js_deps = _closure_js_deps closure_lang_file = _closure_lang_file closure_test_suite = _closure_test_suite mocha_test = _mocha_test diff --git a/javascript/ie-driver/BUILD.bazel b/javascript/ie-driver/BUILD.bazel index 80d7c7548b8df..ada49881dc71b 100644 --- a/javascript/ie-driver/BUILD.bazel +++ b/javascript/ie-driver/BUILD.bazel @@ -1,5 +1,5 @@ -load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_deps", "closure_js_library") -load("//javascript:defs.bzl", "closure_fragment", "closure_lang_file") +load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") +load("//javascript:defs.bzl", "closure_fragment", "closure_js_deps", "closure_lang_file") closure_js_deps( name = "deps", diff --git a/javascript/private/BUILD.bazel b/javascript/private/BUILD.bazel index 34e1392a9e2a4..6a10c6c83f54e 100644 --- a/javascript/private/BUILD.bazel +++ b/javascript/private/BUILD.bazel @@ -1,3 +1,4 @@ +load("@npm//javascript/atoms:google-closure-deps/package_json.bzl", closure_bin = "bin") load("@rules_python//python:defs.bzl", "py_binary") py_binary( @@ -9,3 +10,8 @@ py_binary( "//visibility:public", ], ) + +closure_bin.closure_make_deps_binary( + name = "closure_make_deps", + visibility = ["//javascript:__subpackages__"], +) diff --git a/javascript/private/closure_js_deps.bzl b/javascript/private/closure_js_deps.bzl new file mode 100644 index 0000000000000..971b1c27500d8 --- /dev/null +++ b/javascript/private/closure_js_deps.bzl @@ -0,0 +1,117 @@ +ClosureJsSrcsInfo = provider(fields = ["files"]) + +def _closure_js_srcs_aspect_impl(target, ctx): + """Aspect to collect transitive .js source files from closure_js_library targets.""" + direct = [] + if ctx.rule.kind == "closure_js_library" and hasattr(ctx.rule.attr, "srcs"): + for s in ctx.rule.attr.srcs: + for f in s.files.to_list(): + # Only include .js files from the main workspace, not external repos + if f.extension == "js" and not f.owner.workspace_name: + direct.append(f) + + trans = [] + for d in getattr(ctx.rule.attr, "deps", []): + if ClosureJsSrcsInfo in d: + trans.append(d[ClosureJsSrcsInfo].files) + + return [ClosureJsSrcsInfo(files = depset(direct = direct, transitive = trans))] + +closure_js_srcs_aspect = aspect( + implementation = _closure_js_srcs_aspect_impl, + attr_aspects = ["deps"], +) + +def _collect_srcs_impl(ctx): + """Rule to collect all transitive JS sources from closure_js_library deps.""" + trans = [] + for d in ctx.attr.deps: + if ClosureJsSrcsInfo in d: + trans.append(d[ClosureJsSrcsInfo].files) + all_files = depset(transitive = trans) + return [ + DefaultInfo(files = all_files), + ClosureJsSrcsInfo(files = all_files), + ] + +collect_closure_js_srcs = rule( + implementation = _collect_srcs_impl, + attrs = { + "deps": attr.label_list(aspects = [closure_js_srcs_aspect]), + }, + provides = [ClosureJsSrcsInfo], +) + +def _closure_js_deps_wrapper_impl(ctx): + """Wrapper rule that adds runfiles to the generated deps.js file.""" + deps_file = ctx.file.deps_file + srcs_depset = ctx.attr.srcs_collector[ClosureJsSrcsInfo].files + + return [ + DefaultInfo( + files = depset([deps_file]), + runfiles = ctx.runfiles( + files = [deps_file], + transitive_files = srcs_depset, + ), + ), + ] + +_closure_js_deps_wrapper = rule( + implementation = _closure_js_deps_wrapper_impl, + attrs = { + "deps_file": attr.label(allow_single_file = True, mandatory = True), + "srcs_collector": attr.label(mandatory = True), + }, +) + +def closure_js_deps(name, deps = [], testonly = None, **kwargs): + """Generate a deps.js file from closure_js_library dependencies. + + This macro replaces the old closure_js_deps rule from rules_closure by using + the closure-make-deps binary from the google-closure-deps npm package. + + Args: + name: Name of the target. The output will be 'deps.js'. + deps: List of closure_js_library targets to analyze for dependencies. + """ + + srcs_collector = name + "_closure_srcs" + collect_closure_js_srcs( + name = srcs_collector, + deps = deps, + testonly = testonly, + visibility = ["//visibility:private"], + ) + + deps_genrule = name + "_genrule" + native.genrule( + name = deps_genrule, + srcs = [":" + srcs_collector], + outs = ["deps.js"], + cmd = """ + export BAZEL_BINDIR=$(BINDIR) && \\ + ln -sf . _main && \\ + FILES="" && \\ + for f in $(SRCS); do \\ + FILES="$$FILES --file $$(pwd)/_main/$$f"; \\ + done && \\ + $(location //javascript/private:closure_make_deps) \\ + --closure-path $$(pwd)/external/com_google_javascript_closure_library/closure \\ + --no-validate \\ + $$FILES \\ + > $@ + """, + tools = ["//javascript/private:closure_make_deps"], + testonly = testonly, + visibility = ["//visibility:private"], + ) + + # Use the wrapper rule to add runfiles to the generated deps.js + _closure_js_deps_wrapper( + name = name, + deps_file = deps_genrule, + srcs_collector = ":" + srcs_collector, + testonly = testonly, + **kwargs + ) diff --git a/javascript/webdriver/BUILD.bazel b/javascript/webdriver/BUILD.bazel index 4523e905354c2..d77ca97befa27 100644 --- a/javascript/webdriver/BUILD.bazel +++ b/javascript/webdriver/BUILD.bazel @@ -1,5 +1,5 @@ -load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_deps", "closure_js_library") -load("//javascript:defs.bzl", "closure_test_suite") +load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") +load("//javascript:defs.bzl", "closure_js_deps", "closure_test_suite") closure_js_library( name = "http", diff --git a/javascript/webdriver/atoms/inject/BUILD.bazel b/javascript/webdriver/atoms/inject/BUILD.bazel index 2e3ac2b393f3c..f74069ab3d655 100644 --- a/javascript/webdriver/atoms/inject/BUILD.bazel +++ b/javascript/webdriver/atoms/inject/BUILD.bazel @@ -1,5 +1,5 @@ -load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_deps", "closure_js_library") -load("//javascript:defs.bzl", "closure_fragment") +load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") +load("//javascript:defs.bzl", "closure_fragment", "closure_js_deps") package( default_visibility = [ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c429633131d08..33c851daa4229 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8,6 +8,12 @@ importers: .: {} + javascript/atoms: + devDependencies: + google-closure-deps: + specifier: ^20230802.0.0 + version: 20230802.0.0 + javascript/grid-ui: dependencies: '@apollo/client': @@ -4180,6 +4186,14 @@ packages: slash: 3.0.0 dev: true + /google-closure-deps@20230802.0.0: + resolution: {integrity: sha512-5xhXO8xgxdXYZMsc8ZRzmnCm4prm0TcD33tcNr8l3OjSBovFO9MpgtrZxH/ZVnZMny6oOBmdeNqcO3afCYY9Lw==} + hasBin: true + dependencies: + minimatch: 3.1.2 + yargs: 16.2.0 + dev: true + /gopd@1.2.0: resolution: {integrity: sha512-ZUKRh6/kUFoAiTAtTYPZJ3hw9wNxx+BIBOijnlG9PnrJsCcSjs1wyyD6vJpaYtgnzDrKYRSqf3OO6Rfa93xsRg==} engines: {node: '>= 0.4'} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 7a34df71649ff..30ad894652afa 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,3 +1,4 @@ packages: + - 'javascript/atoms' - 'javascript/grid-ui' - 'javascript/selenium-webdriver'