Skip to content

fix(cdp-client): type-only emitter cast for Page.loadEventFired race - #22

Merged
RapierCraft merged 1 commit into
RapierCraft:mainfrom
sergei-aronsen:fix/cdp-load-event-types
Jun 22, 2026
Merged

fix(cdp-client): type-only emitter cast for Page.loadEventFired race#22
RapierCraft merged 1 commit into
RapierCraft:mainfrom
sergei-aronsen:fix/cdp-load-event-types

Conversation

@sergei-aronsen

Copy link
Copy Markdown
Contributor

Summary

Main currently fails tsc with:

```
src/cdp-client.ts(1394,22): error TS2339: Property 'removeListener' does not exist on type 'Client'.
src/cdp-client.ts(1397,20): error TS2339: Property 'once' does not exist on type 'Client'.
```

Introduced by #18 (ebf055e). The CRI `Client` is an EventEmitter at runtime, but `@types/chrome-remote-interface` only declares `on(...)` — not `once`/`removeListener`. Build fails on every PR until this lands.

Fix

Locally-scoped EventEmitter-shaped interface + a single as unknown as ... cast. No runtime change.

Why it matters

CI is red on main, which means every open PR (#14, #15, #17, #20) is also red regardless of its own correctness. This unblocks all four.

Test plan

  • npm run test:unit (55/55 pass locally)
  • npx tsc --noEmit clean

🤖 Generated with Claude Code

`navigateWithRetry` subscribes to `Page.loadEventFired` via
`client.once` and removes the listener via `client.removeListener` if
the timeout branch wins. The CRI Client is an EventEmitter at runtime
so both methods exist, but @types/chrome-remote-interface@^0.34.x
only declares `on(...)`. tsc therefore rejected this file with:

  src/cdp-client.ts(1394,22): error TS2339: Property 'removeListener'
    does not exist on type 'Client'.
  src/cdp-client.ts(1397,20): error TS2339: Property 'once'
    does not exist on type 'Client'.

The error is real — it broke the build on main itself, blocking every
in-flight PR from passing CI. Fix: a minimal locally-scoped
EventEmitter-shaped interface and a single `as unknown as ...` cast.
No runtime change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sergei-aronsen

Copy link
Copy Markdown
Contributor Author

Heads-up: this PR unblocks four others that are mergeable but red on CI for the same tsc error:

Suggested merge order: this PR first, then the rebased four. Once this lands, hitting "Update branch" on each (or I can force-push an empty rebase to re-trigger CI) flips them green.

@RapierCraft

Copy link
Copy Markdown
Owner

Review: PR #22fix(cdp-client): type-only emitter cast for Page.loadEventFired race

Reviewed commit: dd778ee | Review type: Correctness
Verdict: APPROVE — merge ASAP, unblocks all other PRs


Analysis

  • TypeScript compiles clean (tsc --noEmit exits 0) — this fixes the pre-existing build failure on main
  • The CdpEmitter interface is correctly scoped inside the method block — minimal surface, no module-level pollution
  • as unknown as CdpEmitter is the standard two-step cast for widening then narrowing — correct TypeScript pattern
  • The interface exposes only once and removeListener — exactly what's needed, nothing more
  • No runtime change — the CRI Client already has these methods at runtime, this just makes TypeScript aware

Priority

This should be merged first among all open PRs. Every other PR shows type errors from this same location, making CI red across the board. Merging this unblocks CI for PRs #14, #15, #17, #20, #23.


Review complete. TypeScript: clean build. 0 findings.

@RapierCraft
RapierCraft merged commit d409a06 into RapierCraft:main Jun 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants