Skip to content

[build] Fix Bazel JSDocs implementation#16949

Merged
titusfortner merged 3 commits intotrunkfrom
pr/bazel-jsdocs-fix
Jan 19, 2026
Merged

[build] Fix Bazel JSDocs implementation#16949
titusfortner merged 3 commits intotrunkfrom
pr/bazel-jsdocs-fix

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 19, 2026

User description

💥 What does this PR do?

  • Fix JavaScript documentation generation in Bazel

🔧 Implementation Notes

This fix was verified successful during yesterday's release

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Fix Bazel jsdoc rule to properly reference npm-managed jsdoc binary

  • Update jsdoc templates to output to workspace build directory

  • Add verification that documentation was successfully generated

  • Correct JSDoc type annotation for optional partition key property


Diagram Walkthrough

flowchart LR
  A["jsdoc rule"] -->|"uses npm binary"| B["jsdoc_binary attribute"]
  B -->|"references"| C["npm jsdoc package"]
  A -->|"executes"| D["Unix/Windows template"]
  D -->|"outputs to"| E["build/docs/api/javascript"]
  D -->|"verifies"| F["docs generated successfully"]
Loading

File Walkthrough

Relevant files
Bug fix
jsdoc.bzl
Refactor jsdoc rule templates and add binary attribute     

javascript/private/jsdoc.bzl

  • Add jsdoc_binary attribute to rule definition for npm-managed jsdoc
    binary
  • Update Unix template to use proper runfiles path and output to
    workspace build directory
  • Update Windows template with equivalent path handling and output
    directory
  • Add verification logic to check if documentation was successfully
    generated
  • Set BAZEL_BINDIR environment variable to suppress rules_js errors
  • Handle jsdoc exit code gracefully since it exits 1 on type warnings
+48/-5   
Configuration changes
BUILD.bazel
Wire npm jsdoc binary to docs rule target                               

javascript/selenium-webdriver/BUILD.bazel

  • Load jsdoc binary from npm package using package_json.bzl
  • Create internal jsdoc_bin target wrapping the npm jsdoc binary
  • Pass jsdoc_binary attribute to jsdoc rule invocation
  • Add documentation comments explaining the docs rule behavior
+11/-0   
Documentation
storage.js
Fix JSDoc type annotation for optional property                   

javascript/selenium-webdriver/bidi/storage.js

  • Update JSDoc @returns type annotation for getCookies method
  • Change partitionKey?: PartitionKey to partitionKey:
    (PartitionKey|undefined) for proper type documentation
+1/-1     

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 19, 2026

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: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

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: 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: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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

@selenium-ci selenium-ci added C-nodejs JavaScript Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Jan 19, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 19, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Avoid writing build artifacts into the source tree

The Bazel rule should not write build artifacts like documentation into the
source tree. Instead, it should declare outputs within Bazel's managed
directories using ctx.actions.declare_directory to maintain hermeticity.

Examples:

javascript/private/jsdoc.bzl [33-40]
DEST="$BUILD_WORKSPACE_DIRECTORY/build/docs/api/javascript"
TEMPLATE="$0.runfiles/_main/javascript/selenium-webdriver/node_modules/clean-jsdoc-theme"

# Set BAZEL_BINDIR to suppress rules_js error for non-build actions
export BAZEL_BINDIR="."

# Run jsdoc - ignore exit code since it fails on type warnings
"$0.runfiles/_main/{jsdoc_bin}" --configure {config} --destination "$DEST" --template "$TEMPLATE" "$@" || true
javascript/private/jsdoc.bzl [53-57]
set DEST=%BUILD_WORKSPACE_DIRECTORY%\\build\\docs\\api\\javascript
set TEMPLATE=%~dp0.runfiles\\_main\\javascript\\selenium-webdriver\\node_modules\\clean-jsdoc-theme
set BAZEL_BINDIR=.

"%~dp0.runfiles\\_main\\{jsdoc_bin}" --configure {config} --destination "%DEST%" --template "%TEMPLATE%" %*

Solution Walkthrough:

Before:

# javascript/private/jsdoc.bzl

_UNIX_TEMPLATE = """#!/usr/bin/env bash
...
DEST="$BUILD_WORKSPACE_DIRECTORY/build/docs/api/javascript"
...
# Run jsdoc
jsdoc --destination "$DEST" ...
...
"""

def _jsdoc_impl(ctx):
    # ...
    script = ctx.actions.declare_file(ctx.label.name + ".sh")
    ctx.actions.write(script, _UNIX_TEMPLATE.format(...))
    # ...
    return [DefaultInfo(executable = script, ...)]

After:

# javascript/private/jsdoc.bzl

_UNIX_TEMPLATE = """#!/usr/bin/env bash
...
# DEST is now a path to a declared output directory
DEST="{dest_path}"
...
# Run jsdoc
jsdoc --destination "$DEST" ...
...
"""

def _jsdoc_impl(ctx):
    # ...
    output_dir = ctx.actions.declare_directory(ctx.label.name + "_docs")
    script = ctx.actions.declare_file(ctx.label.name + ".sh")
    ctx.actions.write(
        script,
        _UNIX_TEMPLATE.format(dest_path=output_dir.path, ...),
    )
    # ...
    return [DefaultInfo(executable = script, files = depset([output_dir]), ...)]
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical design flaw introduced in the PR, where a Bazel rule writes outputs to the source tree, breaking hermeticity and potentially causing build and caching issues.

High
Possible issue
Clean destination directory before generating docs
Suggestion Impact:The commit adds cleanup of the docs destination directory before invoking jsdoc, removing any existing DEST and recreating it. It implements this for the UNIX template (rm -rf + mkdir -p) and additionally adds equivalent cleanup for the Windows batch script (rmdir + mkdir).

code diff:

+# Clean destination to prevent stale files
+rm -rf "$DEST"
+mkdir -p "$DEST"
+
 # Run jsdoc - ignore exit code since it fails on type warnings
 "$0.runfiles/_main/{jsdoc_bin}" --configure {config} --destination "$DEST" --template "$TEMPLATE" "$@" || true
 
@@ -53,6 +57,9 @@
 set DEST=%BUILD_WORKSPACE_DIRECTORY%\\build\\docs\\api\\javascript
 set TEMPLATE=%~dp0.runfiles\\_main\\javascript\\selenium-webdriver\\node_modules\\clean-jsdoc-theme
 set BAZEL_BINDIR=.
+
+if exist "%DEST%" rmdir /s /q "%DEST%"
+mkdir "%DEST%"
 

In the _UNIX_TEMPLATE, clean the destination directory before running jsdoc to
prevent stale files from causing incorrect success reports. Add rm -rf "$DEST"
and mkdir -p "$DEST" before the jsdoc command.

javascript/private/jsdoc.bzl [30-49]

 _UNIX_TEMPLATE = """#!/usr/bin/env bash
 set -euo pipefail
 cd "$BUILD_WORKSPACE_DIRECTORY/javascript/selenium-webdriver"
 DEST="$BUILD_WORKSPACE_DIRECTORY/build/docs/api/javascript"
 TEMPLATE="$0.runfiles/_main/javascript/selenium-webdriver/node_modules/clean-jsdoc-theme"
+
+# Clean destination directory to ensure verification is reliable
+rm -rf "$DEST"
+mkdir -p "$DEST"
 
 # Set BAZEL_BINDIR to suppress rules_js error for non-build actions
 export BAZEL_BINDIR="."
 
 # Run jsdoc - ignore exit code since it fails on type warnings
 "$0.runfiles/_main/{jsdoc_bin}" --configure {config} --destination "$DEST" --template "$TEMPLATE" "$@" || true
 
 # Verify docs were generated
 if [[ -d "$DEST" ]] && [[ -n "$(ls -A "$DEST" 2>/dev/null)" ]]; then
     echo "Documentation generated successfully at $DEST"
 else
     echo "ERROR: Documentation was not generated"
     exit 1
 fi
 """

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a flaw in the verification logic where stale files from a previous run could lead to a false positive. Cleaning the destination directory before generation makes the script more robust.

Medium
General
Create DEST directory on Windows

In the _WINDOWS_TEMPLATE, ensure the destination directory exists before running
jsdoc by adding if not exist "%DEST%" mkdir "%DEST%" after setting the DEST
variable.

javascript/private/jsdoc.bzl [53]

 set DEST=%BUILD_WORKSPACE_DIRECTORY%\build\docs\api\javascript
+if not exist "%DEST%" mkdir "%DEST%"

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: This suggestion correctly points out that the Windows script does not create the destination directory, which could cause jsdoc to fail. Adding a command to create the directory improves the script's reliability on Windows.

Low
  • Update

Copy link
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

This PR fixes JavaScript documentation generation in Bazel by correcting JSDoc syntax and properly configuring the jsdoc binary. The fix was verified during yesterday's release.

Changes:

  • Fixed JSDoc type annotation syntax for optional properties to use union types instead of TypeScript-style optional syntax
  • Added jsdoc binary configuration to Bazel build rules
  • Updated jsdoc.bzl rule to accept and use the configured jsdoc binary

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
javascript/selenium-webdriver/bidi/storage.js Fixed JSDoc return type syntax from partitionKey?: PartitionKey to partitionKey: (PartitionKey|undefined) for proper JSDoc parsing
javascript/selenium-webdriver/BUILD.bazel Added jsdoc binary import and configuration, passing jsdoc_binary parameter to the jsdoc rule
javascript/private/jsdoc.bzl Enhanced jsdoc rule to accept jsdoc_binary parameter, use it in generated scripts, and merge its runfiles for proper execution

Copy link
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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 B-devtools Includes everything BiDi or Chrome DevTools related C-nodejs JavaScript Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants