Skip to content

fix(js): migrate closure_js_binary provider access to modern Starlark API#17317

Merged
diemol merged 2 commits intotrunkfrom
fix/closure-js-binary-provider-access
Apr 8, 2026
Merged

fix(js): migrate closure_js_binary provider access to modern Starlark API#17317
diemol merged 2 commits intotrunkfrom
fix/closure-js-binary-provider-access

Conversation

@diemol
Copy link
Copy Markdown
Member

@diemol diemol commented Apr 8, 2026

💥 What does this PR do?

Replace deprecated struct field access with the proper ClosureJsBinaryInfo provider in header.bzl. Bazel 7+ removed the legacy 'field syntax on structs' pattern for provider access.

  • Load ClosureJsBinaryInfo from @rules_closure//closure/private:defs.bzl
  • Replace getattr(d, 'closure_js_binary', None) with ClosureJsBinaryInfo in d
  • Replace d.closure_js_binary.bin with d[ClosureJsBinaryInfo].bin

🔧 Implementation Notes

This fixes local build in IntelliJ with the Bazel plugin

… API

Replace deprecated struct field access with the proper ClosureJsBinaryInfo
provider in header.bzl. Bazel 7+ removed the legacy 'field syntax on structs'
pattern for provider access.
- Load ClosureJsBinaryInfo from @rules_closure//closure/private:defs.bzl
- Replace getattr(d, 'closure_js_binary', None) with ClosureJsBinaryInfo in d
- Replace d.closure_js_binary.bin with d[ClosureJsBinaryInfo].bin
Copilot AI review requested due to automatic review settings April 8, 2026 15:52
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Migrate closure_js_binary provider access to modern Starlark API

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Migrate closure_js_binary provider access to modern Starlark API
• Replace deprecated struct field access with ClosureJsBinaryInfo provider
• Load ClosureJsBinaryInfo from @rules_closure//closure/private:defs.bzl
• Fix Bazel 7+ compatibility in header.bzl

Grey Divider

File Changes

1. javascript/private/header.bzl 🐞 Bug fix +4/-2

Update provider access to modern Starlark API

• Added import of ClosureJsBinaryInfo from @rules_closure//closure/private:defs.bzl
• Replaced deprecated getattr(d, "closure_js_binary", None) check with ClosureJsBinaryInfo in d
• Replaced deprecated d.closure_js_binary.bin access with d[ClosureJsBinaryInfo].bin
• Updated provider access pattern to comply with Bazel 7+ requirements

javascript/private/header.bzl


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Apr 8, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ⚙ Maintainability (1)

Grey Divider


Remediation recommended

1. Loads private rules_closure API 🐞
Description
javascript/private/header.bzl now loads ClosureJsBinaryInfo from
@rules_closure//closure/private:defs.bzl, coupling Selenium to rules_closure’s internal file
layout and visibility rules. This is inconsistent with other Bazel files in the repo that load from
the public @rules_closure//closure:defs.bzl entrypoint and can cause future build breakage when
rules_closure is upgraded or reorganized.
Code

javascript/private/header.bzl[1]

+load("@rules_closure//closure/private:defs.bzl", "ClosureJsBinaryInfo")
Evidence
The PR introduces a load from a closure/private path in header.bzl, while other Selenium
Starlark code uses the public @rules_closure//closure:defs.bzl entrypoint to access rules_closure
APIs. The repo also pins a specific rules_closure version via bzlmod, indicating upgrades are
expected over time; relying on private paths increases upgrade friction.

javascript/private/header.bzl[1-21]
javascript/private/fragment.bzl[1-2]
MODULE.bazel[22-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`javascript/private/header.bzl` loads `ClosureJsBinaryInfo` from a `@rules_closure//closure/private:defs.bzl` path. Depending on private Starlark entrypoints makes upgrades of `rules_closure` more fragile.

## Issue Context
Most other Selenium Bazel code loads from the public `@rules_closure//closure:defs.bzl` entrypoint.

## Fix Focus Areas
- javascript/private/header.bzl[1-2]

## Suggested fix
1) First try switching to the public entrypoint:
```starlark
load("@rules_closure//closure:defs.bzl", "ClosureJsBinaryInfo")
```
2) If `ClosureJsBinaryInfo` is not exported publicly in the pinned `rules_closure` version, introduce a small local shim (e.g., in `javascript/private/closure_providers.bzl`) that does the private load in one place, and load that shim from `header.bzl` with a comment explaining the dependency and what needs updating on future `rules_closure` bumps.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Apr 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the JavaScript Bazel Starlark rule implementation to use modern provider-based access for Closure JS binaries, avoiding deprecated struct-field access that no longer works in Bazel 7+.

Changes:

  • Load ClosureJsBinaryInfo and switch detection from getattr(d, "closure_js_binary", None) to ClosureJsBinaryInfo in d.
  • Switch provider access from d.closure_js_binary.bin to d[ClosureJsBinaryInfo].bin.

@diemol diemol merged commit 375a1fd into trunk Apr 8, 2026
26 checks passed
@diemol diemol deleted the fix/closure-js-binary-provider-access branch April 8, 2026 16:04
AutomatedTester pushed a commit that referenced this pull request Apr 9, 2026
… API (#17317)

Replace deprecated struct field access with the proper ClosureJsBinaryInfo
provider in header.bzl. Bazel 7+ removed the legacy 'field syntax on structs'
pattern for provider access.
- Load ClosureJsBinaryInfo from @rules_closure//closure/private:defs.bzl
- Replace getattr(d, 'closure_js_binary', None) with ClosureJsBinaryInfo in d
- Replace d.closure_js_binary.bin with d[ClosureJsBinaryInfo].bin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants