Skip to content
This repository was archived by the owner on Oct 22, 2025. It is now read-only.

Conversation

@valstu
Copy link

@valstu valstu commented Feb 24, 2025

As discussed in this discussion #621, you get following error when initializing Client on browser environment

Uncaught (in promise) TypeError: WebSocket2 is not a constructor

This most likely happens because on the websocket.ts file we first try to import ws dynamically, if that is successful we never reach the catch block. For some reason importing ws on the browser environment does not throw even though the ws package is not designed to be used on browser.

In this PR I flipped the logic other way around, if typeof Websocket is not defined, browser Websocket implementation is used. If Websocket does not exist we try to import ws.

Let me know if this makes sense or not?

@NathanFlurry NathanFlurry self-requested a review February 24, 2025 17:30
Copy link
Member

@NathanFlurry NathanFlurry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the pr!

interesting that it doesn't throw in a browser env, i wonder if it's a bundler-specific thing with how the import("ws") is handled.

the reason i wrote it this way originally is because node 20 logs an ugly warning saying something like "websockets are experimental" when you attempt to use it, so i decided to default to ws.

i think something like this would correctly fallback in a browser env:

try {
  const ws = await import("ws");
  if (!ws.WebSocket) throw "Unsupported";
  _WebSocket = ws.WebSocket as unknown as typeof WebSocket;
} catch {
  // ...etc...
}

note: if we end up flipping logic, we also need to flip sse.ts.

try {
// Node.js environment
const ws = await import("ws");
console.log('toimiiko tää selaimes', ws)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra log

@valstu
Copy link
Author

valstu commented Feb 24, 2025

thanks for the pr!

interesting that it doesn't throw in a browser env, i wonder if it's a bundler-specific thing with how the import("ws") is handled.

the reason i wrote it this way originally is because node 20 logs an ugly warning saying something like "websockets are experimental" when you attempt to use it, so i decided to default to ws.

i think something like this would correctly fallback in a browser env:

try {
  const ws = await import("ws");
  if (!ws.WebSocket) throw "Unsupported";
  _WebSocket = ws.WebSocket as unknown as typeof WebSocket;
} catch {
  // ...etc...
}

note: if we end up flipping logic, we also need to flip sse.ts.

Thanks for your reply, didn't know about the issue with Node 20 and your solution is closer to original so I switched to that. Seems to work fine locally when I tested this.

@valstu valstu requested a review from NathanFlurry February 25, 2025 21:22
@NathanFlurry
Copy link
Member

@valstu can you test these changes i released in v0.6.3: #700

thanks for pointing this out!

@NathanFlurry
Copy link
Member

Closing unless still broken

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants