[bazel] Switch to custom closure_js_deps rule

[shs96c]

User description

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.

---

PR Type

Enhancement, Tests

---

Description

• Replace deprecated rules_closure closure_js_deps with custom implementation

• Implement custom Bazel rule using google-closure-deps npm package

• Add aspect-based transitive JS source collection from closure_js_library targets

• Update all BUILD files to import closure_js_deps from custom implementation

---

Diagram Walkthrough

flowchart LR
  A["rules_closure removed<br/>closure_js_deps"] -->|"Replace with"| B["Custom closure_js_deps<br/>implementation"]
  B -->|"Uses"| C["google-closure-deps<br/>npm package"]
  C -->|"Generates"| D["deps.js file"]
  B -->|"Collects via"| E["Aspect-based<br/>JS sources"]

Loading

File Walkthrough

	Relevant files
Enhancement	3 files defs.bzl Export custom closure_js_deps rule                                              +2/-0      closure_js_deps.bzl Implement custom closure_js_deps rule with aspects              +117/-0  BUILD.bazel Add closure_make_deps binary target                                            +6/-0
Dependencies	2 files package.json Add google-closure-deps npm dependency                                      +14/-0    pnpm-lock.yaml Add google-closure-deps package lock entry                              +14/-0
Configuration changes	8 files BUILD.bazel Import closure_js_deps from custom implementation                +2/-2      BUILD.bazel Import closure_js_deps from custom implementation                +2/-2      BUILD.bazel Import closure_js_deps from custom implementation                +2/-2      BUILD.bazel Import closure_js_deps from custom implementation                +2/-2      BUILD.bazel Import closure_js_deps from custom implementation                +2/-2      MODULE.bazel Include javascript/atoms package.json in npm lock                +1/-0      .bazelignore Ignore javascript/atoms node_modules directory                      +1/-0      pnpm-workspace.yaml Add javascript/atoms to workspace packages                              +1/-0

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new Bazel rules and macro generate files and invoke external tools without any logging of critical actions, but build tooling may not require runtime audit trails; confirm if build-time actions must be audited. Referred Code 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( ... (clipped 29 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Genrule errors: The native.genrule command pipeline does not include explicit error checks or handling for missing tools/inputs, relying on shell defaults; confirm if Bazel failure propagation is sufficient for your standards. Referred Code 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"], ... (clipped 1 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Tool stderr leak: The macro invokes an external tool via genrule which may print detailed errors; while Bazel logs are developer-facing, verify that no user-facing surface exposes these errors. Referred Code 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"], ... (clipped 1 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: External tool trust: The build integrates google-closure-deps via pnpm and executes it in a genrule without checksum/pin verification beyond lockfile; confirm supply chain validation and restrict inputs to workspace JS files as intended. Referred Code 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( ... (clipped 29 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Refactor genrule into a Starlark rule Replace the genrule and its complex shell script with a proper Starlark rule. Use ctx.actions.run and ctx.actions.args with a parameter file to robustly handle a large number of source files and avoid command-line length limits. Examples: javascript/private/closure_js_deps.bzl [88-108] 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"; \\ ... (clipped 11 lines) Solution Walkthrough: Before: # javascript/private/closure_js_deps.bzl def closure_js_deps(name, deps = [], **kwargs): # ... native.genrule( name = deps_genrule, srcs = [":" + srcs_collector], outs = ["deps.js"], cmd = """ ... FILES="" && \\ for f in $(SRCS); do \\ FILES="$$FILES --file $$(pwd)/_main/$$f"; \\ done && \\ $(location //javascript/private:closure_make_deps) \\ $$FILES \\ > $@ """, tools = ["//javascript/private:closure_make_deps"], # ... ) # ... After: # javascript/private/closure_js_deps.bzl def _closure_js_deps_gen_impl(ctx): args = ctx.actions.args() # Use a param file to avoid command line length limits args.use_param_file("@%s", use_always = True) args.add("--closure-path", "external/com_google_javascript_closure_library/closure/goog") args.add("--no-validate") args.add_all(ctx.files.srcs, before_each = "--file") ctx.actions.run( executable = ctx.executable._tool, arguments = [args], outputs = [ctx.outputs.out], ) _closure_js_deps_gen = rule( implementation = _closure_js_deps_gen_impl, attrs = { "srcs": attr.label_list(allow_files=True), "out": attr.output(), "_tool": attr.label(...) }, ) def closure_js_deps(name, deps = [], **kwargs): # ... # Call the new rule instead of native.genrule _closure_js_deps_gen(...) # ... Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw in the genrule implementation—its shell script is brittle and will fail with many files due to command-line length limits—and proposes the idiomatic and robust Bazel solution using Starlark actions.	High
Possible issue	Avoid exceeding command-line length limit Refactor the genrule command to use xargs and a temporary file for source file arguments. This avoids exceeding the command-line length limit, preventing potential build failures. javascript/private/closure_js_deps.bzl [92-104] cmd = """ export BAZEL_BINDIR=$(BINDIR) && \ ln -sf . _main && \ - FILES="" && \ + SRCS_FILE=$$(mktemp) && \ for f in $(SRCS); do \ - FILES="$$FILES --file $$(pwd)/_main/$$f"; \ + echo "$$(pwd)/_main/$$f" >> $$SRCS_FILE; \ done && \ - $(location //javascript/private:closure_make_deps) \ + xargs -a $$SRCS_FILE -r -- $(location //javascript/private:closure_make_deps) \ --closure-path $$(pwd)/external/com_google_javascript_closure_library/closure \ --no-validate \ - $$FILES \ + --file \ > $@ """, Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential Argument list too long error and proposes a robust, standard solution using xargs to prevent it, which significantly improves the reliability of the build script for projects with many files.	Medium
Learned best practice	Validate external tool inputs Validate that the external closure library path exists and fail fast with a clear message if missing. Also guard empty source lists to prevent generating invalid deps. javascript/private/closure_js_deps.bzl [88-108] 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 \\ - > $@ + set -euo pipefail + export BAZEL_BINDIR=$(BINDIR) + ln -sf . _main + FILES="" + for f in $(SRCS); do + FILES="$$FILES --file $$(pwd)/_main/$$f" + done + if [ -z "$$FILES" ]; then + echo "No JS sources discovered for deps generation" >&2 + exit 1 + fi + CLOSURE_PATH="$$(pwd)/external/com_google_javascript_closure_library/closure" + if [ ! -d "$$CLOSURE_PATH" ]; then + echo "Closure library not found at $$CLOSURE_PATH" >&2 + exit 1 + fi + $(location //javascript/private:closure_make_deps) \ + --closure-path "$$CLOSURE_PATH" \ + --no-validate \ + $$FILES \ + > "$@" """, tools = ["//javascript/private:closure_make_deps"], testonly = testonly, visibility = ["//visibility:private"], ) Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard external tool invocations with validation and clear errors to avoid brittle builds.	Low
	Complete and correct docstring Update the docstring to document parameters like 'testonly' and kwargs behavior to reflect the macro’s actual API. javascript/private/closure_js_deps.bzl [68-77] 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. + Replaces rules_closure's closure_js_deps using the google-closure-deps CLI. Args: - name: Name of the target. The output will be 'deps.js'. - deps: List of closure_js_library targets to analyze for dependencies. + name: Target name; produces an output file 'deps.js'. + deps: List of closure_js_library targets to analyze (transitively). + testonly: Whether the targets are for tests only. + **kwargs: Additional attributes forwarded to the wrapper target (visibility, tags, etc.). + Returns: + A target that exposes 'deps.js' and runfiles containing the analyzed sources. """ Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Enforce accurate and consistent documentation to match actual behavior.	Low
More

[diemol]

Thank you.