[js] Add Javascript/Typescript CDDL code generator for WebDriver BiDi#17574
Open
pujagani wants to merge 1 commit into
Open
[js] Add Javascript/Typescript CDDL code generator for WebDriver BiDi#17574pujagani wants to merge 1 commit into
pujagani wants to merge 1 commit into
Conversation
selenium-ci
added
C-py
Contributor
Review Summary by QodoAdd JavaScript/TypeScript CDDL code generator for WebDriver BiDi with comprehensive test coverage
WalkthroughsDescription• Implements a comprehensive WebDriver BiDi code generator (generate_bidi.mjs) that transforms CDDL specifications into typed TypeScript domain classes • Two-pass generation approach: Pass 1 uses cddl2ts for type declarations with post-processing; Pass 2 walks raw CDDL AST to extract commands and events for implementation • Generates per-domain modules (Browser, BrowsingContext, Input, Log, Network, Script, Storage, Emulation) with typed command methods and event subscription helpers • Introduces enhancements manifest (bidi_enhancements_manifest.json) for injecting hand-written additions (deprecated compatibility shims, convenience wrappers) without modifying generated code • Integrates code generation into Bazel build pipeline via generate_bidi_library() macro that orchestrates CDDL merging, TypeScript generation, and compilation • Comprehensive integration test suite covering all generated BiDi domains with Chrome and Firefox browser validation • Adds cddl@^0.20.1 and cddl2ts@^0.9.1 dev dependencies and updates pnpm workspace configuration for dependency resolution • Expands visibility of CDDL merge tool to JavaScript build pipeline Diagramflowchart LR
CDDL["CDDL Specifications<br/>WebDriver BiDi, Permissions,<br/>Prefetch, UA Client Hints"]
Merge["Merge CDDL<br/>merge_cddl"]
Gen["Two-Pass Generator<br/>generate_bidi.mjs"]
Types["Type Declarations<br/>cddl2ts Pass 1"]
Impl["Implementation Classes<br/>Pass 2 AST Walk"]
Manifest["Enhancements Manifest<br/>bidi_enhancements_manifest.json"]
Output["Generated Domain Modules<br/>Browser, BrowsingContext,<br/>Input, Log, Network,<br/>Script, Storage, Emulation"]
Tests["Integration Tests<br/>All domains + browsers"]
Package["npm Package"]
CDDL --> Merge
Merge --> Gen
Gen --> Types
Gen --> Impl
Manifest --> Gen
Types --> Output
Impl --> Output
Output --> Tests
Output --> Package
Tests -.-> Package
File Changes1. javascript/selenium-webdriver/test/bidi/generated/browsing_context_test.js
|
Contributor
Code Review by Qodo
1. username! in continueWithAuth
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "extraMethods": { | ||
| "cancelAuth": " async cancelAuth(requestId: NetworkRequest): Promise<void> {\n await this.continueWithAuth({ request: requestId, action: 'cancel' } as NetworkContinueWithAuthParameters)\n }", | ||
| "continueWithAuthNoCredentials": " async continueWithAuthNoCredentials(requestId: NetworkRequest): Promise<void> {\n await this.continueWithAuth({ request: requestId, action: 'default' } as NetworkContinueWithAuthParameters)\n }", | ||
| "continueWithAuth": " async continueWithAuth(params: NetworkContinueWithAuthParameters): Promise<void>\n /** @deprecated Pass a NetworkContinueWithAuthParameters object instead — will be removed in a future major version. */\n async continueWithAuth(requestId: NetworkRequest, username: string, password: string): Promise<void>\n async continueWithAuth(\n paramsOrRequestId: NetworkContinueWithAuthParameters | NetworkRequest,\n username?: string,\n password?: string,\n ): Promise<void> {\n const params: NetworkContinueWithAuthParameters =\n typeof paramsOrRequestId === 'string'\n ? ({\n request: paramsOrRequestId,\n action: 'provideCredentials',\n credentials: { type: 'password', username: username!, password: password! },\n } as NetworkContinueWithAuthParameters)\n : (paramsOrRequestId as NetworkContinueWithAuthParameters)\n await this.bidi.send({\n method: 'network.continueWithAuth',\n params: params as unknown as Record<string, unknown>,\n })\n }", |
Contributor
There was a problem hiding this comment.
1. username! in continuewithauth 📘 Rule violation ≡ Correctness
The injected deprecated overload for Network.continueWithAuth uses non-null assertions (username!, password!) on optional parameters, so invalid calls can produce undefined credentials or unclear runtime failures. Protocol-derived/user inputs should be validated and rejected with deterministic, actionable errors.
Agent Prompt
## Issue description
The deprecated overload implementation for `Network.continueWithAuth` uses `username!`/`password!` non-null assertions even though the implementation signature allows them to be omitted, which can lead to undefined credentials being sent or confusing runtime errors.
## Issue Context
This code is injected from the enhancements manifest and becomes part of the published/generated JS/TS API surface.
## Fix Focus Areas
- javascript/selenium-webdriver/private/bidi_enhancements_manifest.json[51-51]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+13
| "back": " async back(): Promise<void> {\n await this.traverseHistory({ context: '', delta: -1 } as BrowsingContextTraverseHistoryParameters)\n }", | ||
| "forward": " async forward(): Promise<void> {\n await this.traverseHistory({ context: '', delta: 1 } as BrowsingContextTraverseHistoryParameters)\n }", |
Contributor
There was a problem hiding this comment.
2. Empty context in back/forward 🐞 Bug ≡ Correctness
The BrowsingContext enhancement methods back() and forward() always call traverseHistory with context: '' (empty string), so these convenience methods will consistently fail with an invalid browsing context id.
Agent Prompt
### Issue description
`back()`/`forward()` in the enhancements manifest hard-code `context: ''` and therefore cannot work against any BiDi server.
### Issue Context
The generated API is params-object based; these helpers must either accept a context id or be removed until a proper abstraction exists.
### Fix Focus Areas
- javascript/selenium-webdriver/private/bidi_enhancements_manifest.json[8-16]
### Suggested fix
- Change the shim signatures to accept a context id:
- `async back(context: BrowsingContextBrowsingContext): Promise<void>`
- `async forward(context: BrowsingContextBrowsingContext): Promise<void>`
- Delegate to `traverseHistory({ context, delta: ±1 })` (no empty string).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+49
to
+51
| "cancelAuth": " async cancelAuth(requestId: NetworkRequest): Promise<void> {\n await this.continueWithAuth({ request: requestId, action: 'cancel' } as NetworkContinueWithAuthParameters)\n }", | ||
| "continueWithAuthNoCredentials": " async continueWithAuthNoCredentials(requestId: NetworkRequest): Promise<void> {\n await this.continueWithAuth({ request: requestId, action: 'default' } as NetworkContinueWithAuthParameters)\n }", | ||
| "continueWithAuth": " async continueWithAuth(params: NetworkContinueWithAuthParameters): Promise<void>\n /** @deprecated Pass a NetworkContinueWithAuthParameters object instead — will be removed in a future major version. */\n async continueWithAuth(requestId: NetworkRequest, username: string, password: string): Promise<void>\n async continueWithAuth(\n paramsOrRequestId: NetworkContinueWithAuthParameters | NetworkRequest,\n username?: string,\n password?: string,\n ): Promise<void> {\n const params: NetworkContinueWithAuthParameters =\n typeof paramsOrRequestId === 'string'\n ? ({\n request: paramsOrRequestId,\n action: 'provideCredentials',\n credentials: { type: 'password', username: username!, password: password! },\n } as NetworkContinueWithAuthParameters)\n : (paramsOrRequestId as NetworkContinueWithAuthParameters)\n await this.bidi.send({\n method: 'network.continueWithAuth',\n params: params as unknown as Record<string, unknown>,\n })\n }", |
Contributor
There was a problem hiding this comment.
3. Continuewithauth ignores errors 🐞 Bug ≡ Correctness
The Network enhancement overrides continueWithAuth() and calls this.bidi.send() directly without checking for BiDi error responses, so server-side errors are silently ignored instead of thrown like other generated commands.
Agent Prompt
### Issue description
The `continueWithAuth` enhancement override bypasses the generator’s standard `response.type === 'error'` check. Since `bidi.send()` resolves with the raw payload, this makes failures silent.
### Issue Context
`generate_bidi.mjs` intentionally makes generated commands throw on `response['type'] === 'error'`. Overriding a command should preserve this behavior.
### Fix Focus Areas
- javascript/selenium-webdriver/private/bidi_enhancements_manifest.json[45-57]
- javascript/selenium-webdriver/generate_bidi.mjs[860-891]
- javascript/selenium-webdriver/bidi/index.js[177-208]
### Suggested fix
- In the override, capture the response from `this.bidi.send(...)` and apply the same error check pattern as generated methods.
- If `response.type === 'error'`, throw an `Error` with the server-provided details.
- Alternatively, avoid overriding the generated `continueWithAuth(params)` method:
- Keep the generated method intact.
- Add a separate deprecated wrapper name for the legacy positional signature that calls the generated method with a constructed params object.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
2 participants
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.
🔗 Related Issues
Part of the CDDL WebDriver BiDi API generator. Implemented as defined in the spec https://gist.github.com/AutomatedTester/148437f846a3759f7678c7ff6debb210
💥 What does this PR do?
Adds a code generator (
generate_bidi.mjs) that reads the merged WebDriver BiDi CDDLspecification and emits one typed TypeScript module per domain into
bidi/generated/.The generator is wired into the Bazel build so the output is compiled and included in
the npm package automatically.
Each generated domain class (
Browser,BrowsingContext,Input,Log,Network,Script,Storage, etc.) exposes typed command methods and event subscription helpersvia a
static async create(driver)factory. An enhancements manifest(
private/bidi_enhancements_manifest.json) allows hand-written additions (deprecatedcompatibility shims, convenience wrappers, and method overloads) to be injected at
build time without modifying generated files.
🔧 Implementation Notes
Why a two-pass generator?
cddl2tsonly produces type declarations; it has no concept of the BiDi command/eventdistinction. A second pass walks the raw CDDL AST to identify commands (groups with a
methodliteral andparams) and events (same structure plustype: "event") andemits the implementation class from that.
Why throw on error responses?
bidi/index.jsalways resolves — it never rejects on BiDi error responses. Both voidand non-void generated methods inspect the response payload and throw if
response.type === 'error', so unhandled server errors surface as test failures ratherthan being silently swallowed.
Why an enhancements manifest?
The existing hand-crafted modules use a positional-argument / builder pattern
(
getBrowserInstance(driver),AddInterceptParameters). The generated API uses typedparams objects. Replacing the old exports directly would be a breaking change. The
manifest lets deprecated compatibility wrappers live alongside generated code until a
major version bump removes them.
🤖 AI assistance
integration tests under
test/bidi/generated/💡 Additional Considerations
Breaking change: The generated API shape (
Domain.create(driver), typed paramsobjects) differs from the existing hand-crafted modules (positional args, builder
classes). Deprecated compatibility shims are in place for the most-used modules but a
full audit is needed before the old modules can be removed. See the Phase 7 audit in
the PR description or tracking issue for the complete list.
Follow-on work required:
captureBoxScreenshot,captureElementScreenshot,locateNode,locateElement,locateElements(BrowsingContext); filtered log level variants (Log).browsingContextInspector.jsandnetworkInspector.js— superseded byevent methods on the generated
BrowsingContextandNetworkclassesAddInterceptParameters,ContinueRequestParameters,CookieFilter, etc.).🔄 Types of changes