fix(WebSocketFrame): allow for broader connection types

[kettanaito]

• Fixes [experimental] WebSocketNetworkFrame cannot be initialized with WebSocket(Client|Server)ConnectionProtocol implementations #2707

I'm a bit tentative about this approach.

• Find a proper name for WebSocketConnectionFoo.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce generics to WebSocketNetworkFrame to accept WebSocketConnectionProtocol implementations. A new WebSocketConnectionFoo interface is created as an abstract base for WebSocket connection data. Related APIs, handlers, and utilities are updated to support both concrete and protocol-based connections with runtime guards.

Changes

Cohort / File(s)	Summary
WebSocketNetworkFrame Generics src/core/experimental/frames/websocket-frame.ts	Removed hard dependency on @mswjs/interceptors. Made WebSocketNetworkFrameOptions and WebSocketNetworkFrame generic with constraint Connection extends WebSocketConnectionFoo, replacing fixed WebSocketConnectionData type.
WebSocketConnectionFoo Abstraction src/core/handlers/WebSocketHandler.ts, src/core/experimental/sources/interceptor-source.ts	Introduced new WebSocketConnectionFoo interface with shared connection fields. Redefined WebSocketHandlerConnection to extend it. Updated handler methods to accept WebSocketConnectionFoo. Updated InterceptorWebSocketNetworkFrame to explicitly bind generic parameter.
Logger Runtime Guards src/core/ws/utils/attachWebSocketLogger.ts	Broadened attachWebSocketLogger and logConnectionOpen to accept union of WebSocketConnectionData and WebSocketConnectionFoo. Added runtime guards checking for socket existence before registering listeners and accessing socket properties.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A frame so generic, no longer confined,
Protocol-based connections of every kind,
Guards check for sockets with cautious care,
WebSocketConnectionFoo shared everywhere!
Flexibility springs forth, both concrete and pure,
An abstraction that's flexible, elegant, and sure. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(WebSocketFrame): allow for broader connection types' is directly related to the primary change in the changeset, which introduces generics to allow WebSocketNetworkFrame to accept broader connection types instead of only WebSocketConnectionData.
Linked Issues check	✅ Passed	The PR successfully addresses issue #2707 by making WebSocketNetworkFrame generic over Connection types, introducing WebSocketConnectionFoo base interface, and updating handler signatures to accept broader protocol implementations instead of requiring concrete WebSocketConnectionData types.
Out of Scope Changes check	✅ Passed	All changes are scoped to widening connection type acceptance in WebSocketNetworkFrame and related handlers. Updates to attachWebSocketLogger and InterceptorWebSocketNetworkFrame are necessary to support the new generic approach and do not introduce unrelated functionality.
Description check	✅ Passed	The description references issue #2707 and indicates changes to WebSocketFrame connection types, which aligns with the changeset that broadens connection type constraints.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/websocket-frame-connection-type

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[kettanaito]

@christoph-fricke, hi 👋 What do you think about this approach? Do you see any potential issues with this?

[coderabbitai]

🧹 Nitpick comments (1)

src/core/handlers/WebSocketHandler.ts (1)

25-33: Rename WebSocketConnectionFoo before it becomes API baggage.

Foo looks like a placeholder. Since this type is exported and now reused across the frame/logger surface, a descriptive name like WebSocketConnectionLike or BaseWebSocketConnection will age much better.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/handlers/WebSocketHandler.ts` around lines 25 - 33, The exported
interface name WebSocketConnectionFoo is a placeholder and should be renamed to
a descriptive stable name used across the surface; rename WebSocketConnectionFoo
to something like WebSocketConnectionLike or BaseWebSocketConnection and update
all references (including the extending type WebSocketHandlerConnection) so the
export and internal uses compile and convey intent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/core/handlers/WebSocketHandler.ts`:
- Around line 25-33: The exported interface name WebSocketConnectionFoo is a
placeholder and should be renamed to a descriptive stable name used across the
surface; rename WebSocketConnectionFoo to something like WebSocketConnectionLike
or BaseWebSocketConnection and update all references (including the extending
type WebSocketHandlerConnection) so the export and internal uses compile and
convey intent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ca97741a-f57b-40c5-bbf0-a5eb2f79e472

📥 Commits

Reviewing files that changed from the base of the PR and between 33bf349 and 69c7ad8.

📒 Files selected for processing (4)

• src/core/experimental/frames/websocket-frame.ts
• src/core/experimental/sources/interceptor-source.ts
• src/core/handlers/WebSocketHandler.ts
• src/core/ws/utils/attachWebSocketLogger.ts

[christoph-fricke]

@kettanaito I packed it locally and tested it with @msw/playwright. With this, the existing protocol implementations work and the tests pass. Just had to add a wonky import to node_modules/../../WebSocketHandler to get to the WebSocketConnectionFoo type.

Not too sure what I think about the implementation. It feels a bit "bolted on", like trying to squeeze the protocol interfaces in afterwards. But that might be just me knowing the history...
Will it cause big observable differences between sources with WebSocketConnectionFoo and sources with WebSocketConnectionData?

Left an additional comment in #2707 that might make this PR obsolete.

[kettanaito]

The dichotomy comes from the frames not guaranteeing to be created by the WebSocketInterceptor anymore. We have that assumption right now and it's rather strong (nothing wrong with that since ws works via the interceptor). But with custom network sources, they can yield whichever frames they'd like. Architecture-wise the Foo class makes perfect sense.

The union with Data class is there only for posterity. The ws functionality, effectively, has to branch since now WebSocketFrame can have a connection that wasn't created by the interceptor and, as a result, doesn't have client.socket and, potentially, other stuff. This is a bit troubling as I cannot predict whether our internal ws (and what developers expect from it) and user-defined WebSocketFrames won't conflict/miss in behaviors.

[christoph-fricke]

As far as I understand, the ConnectionProtocols interfaces are the intended abstraction for decoupling MSW internals from WebSocket specifics, right? They are supposed to enable custom frames that extend can extend WebSocketNetworkFrame.
The previous @msw/playwright implementation already proofed that it is possible to implement ConnectionProtocols with something that is not a WebSocketInterceptor. For other alternative sources, I am toying with the idea of a HttpServerSource and WebSocketServerSource, i.e. sources that create an actual server. I have some use cases for that... I don't see blockers creating ConnectionProtocols there either.

The biggest change I would make: I think ConnectionProtocols as the abstraction target is better "highlighted" and "communicated" when the WebSocketNetworkFrame does not accept an generic connection. It feels like overhead and is just one more think to understand. To avoid more confusion, it might help if these interfaces do not need imports from @mswjs/interceptors, but that might be unfeasible. In the end, most people will likely just use existing sources and not implement their own sources and frames... Adjusted frame options:

interface WebSocketNetworkFrameOptions {
  connection: WebSocketConnectionFoo // <-- Type must be package exported
}

// I actually prefer it flattened (avoids having to find a name for "Foo"):
interface WebSocketNetworkFrameOptions {
  client: WebSocketClientConnectionProtocol
  server: WebSocketServerConnectionProtocol
  info: WebSocketConnectionData['info']
}

I quickly verify that removing the generic works. It only requires a small adjustment to the InterceptorWebSocketNetworkFrame:

class InterceptorWebSocketNetworkFrame extends WebSocketNetworkFrame {
  #client: WebSocketConnectionData['client']

  constructor(args: { connection: WebSocketConnectionData }) {
    super({ connection: args.connection })
    this.#client = args.connection.client
  }

  public errorWith(reason?: unknown): void {
     // ... collapsed code
     this.#client.socket.dispatchEvent(errorEvent)
  }

  public passthrough() {
    this.data.connection.server.connect()
  }
}