fix: relax bidi runtime typing for direct pnpm #152
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new WebDriver BiDi package (@effect-native/bidi) that provides runtime primitives for the WebDriver BiDi protocol within the Effect ecosystem. The implementation relaxes typing constraints to use unknown for command results, adds effect as a workspace dev dependency, and establishes the foundational infrastructure for BiDi command/response correlation and event handling.
Key changes:
- Creates a complete BiDi package with command routing, event subscription, and transport abstraction
- Simplifies service typing by using
unknownfor command results to avoid unsafe type assertions - Adds workspace-level
effectdev dependency to resolve peer dependency issues outside nix shell
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages-native/bidi/src/internal/BiDi.ts |
Core BiDi implementation with transport abstraction, command correlation, and event dispatching |
packages-native/bidi/src/BiDi.ts |
Public API surface re-exporting internal types and services with WebDriver BiDi spec documentation |
packages-native/bidi/test/BiDi.test.ts |
Test suite validating command/response correlation and error handling scenarios |
packages-native/bidi/package.json |
Package configuration with optional effect peer dependency and workspace dev dependency |
package.json |
Adds workspace-level effect dev dependency for BiDi peer resolution |
.specs/bidi/progress.md |
Implementation roadmap tracking BiDi specification coverage and milestone progress |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| export type EventHandler = (event: Event) => Effect.Effect<void> | ||
|
|
||
| /** @internal */ | ||
| export interface BiDiService { | ||
| readonly send: <Params>( | ||
| command: Command<Params> | ||
| ) => Effect.Effect<unknown, CommandFailed | TransportSendError | TransportClosed> | ||
| readonly onEvent: (method: string, handler: EventHandler) => Effect.Effect<void, never, Scope.Scope> |
There was a problem hiding this comment.
The EventHandler type lacks error handling capability. Consider allowing handlers to fail with specific error types: (event: Event) => Effect.Effect<void, E> to enable proper error propagation from event processing.
| export type EventHandler = (event: Event) => Effect.Effect<void> | |
| /** @internal */ | |
| export interface BiDiService { | |
| readonly send: <Params>( | |
| command: Command<Params> | |
| ) => Effect.Effect<unknown, CommandFailed | TransportSendError | TransportClosed> | |
| readonly onEvent: (method: string, handler: EventHandler) => Effect.Effect<void, never, Scope.Scope> | |
| export type EventHandler<E = never> = (event: Event) => Effect.Effect<void, E> | |
| /** @internal */ | |
| export interface BiDiService { | |
| readonly send: <Params>( | |
| command: Command<Params> | |
| ) => Effect.Effect<unknown, CommandFailed | TransportSendError | TransportClosed> | |
| readonly onEvent: <E = never>(method: string, handler: EventHandler<E>) => Effect.Effect<void, E, Scope.Scope> |
| if ((failure as { readonly _tag?: string })._tag !== "CommandFailed") { | ||
| throw new Error(`unexpected failure: ${JSON.stringify(failure)}`) | ||
| } |
There was a problem hiding this comment.
Using type assertion with manual tag checking is fragile. Consider using Effect's pattern matching utilities or a proper type guard function to check the failure type more safely.
| Object.prototype.hasOwnProperty.call(message, "id") && | ||
| Object.prototype.hasOwnProperty.call(message, "result") | ||
|
|
||
| const isCommandError = (message: IncomingMessage): message is CommandError => | ||
| Object.prototype.hasOwnProperty.call(message, "id") && | ||
| Object.prototype.hasOwnProperty.call(message, "error") |
There was a problem hiding this comment.
Using Object.prototype.hasOwnProperty.call() is verbose and can be replaced with the more modern Object.hasOwn() method for better readability and performance.
| Object.prototype.hasOwnProperty.call(message, "id") && | |
| Object.prototype.hasOwnProperty.call(message, "result") | |
| const isCommandError = (message: IncomingMessage): message is CommandError => | |
| Object.prototype.hasOwnProperty.call(message, "id") && | |
| Object.prototype.hasOwnProperty.call(message, "error") | |
| Object.hasOwn(message, "id") && | |
| Object.hasOwn(message, "result") | |
| const isCommandError = (message: IncomingMessage): message is CommandError => | |
| Object.hasOwn(message, "id") && | |
| Object.hasOwn(message, "error") |
subtleGradient
changed the base branch from
effect-native/main
to
codex/create-@effect-native/bidi-package-with-tdd
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68d6f9e082f8832f82a4ce3673d79f64