yjs / y-websocket

Websocket Connector for Yjs
https://docs.yjs.dev/ecosystem/connection-provider/y-websocket
MIT License
492 stars 255 forks source link

Version 1.5.2 breaks y-websocket in the browser #166

Closed kevinvalk closed 7 months ago

kevinvalk commented 7 months ago

Describe the bug On new WebsocketProvider the following error is thrown.

y-websocket.js?v=f1f96d6f:381 Uncaught (in promise) TypeError: process.on is not a function
    at new WebsocketProvider (y-websocket.js?v=f1f96d6f:381:15)

To Reproduce

Expected behavior No error

Screenshots If applicable, add screenshots to help explain your problem.

Environment Information

Additional context So in commit 882ac9eb79593ef1ecf6c687e97b26865c82878c the logic for (if I read correctly) detecting if it is running in a browser was removed. It is weird, because the original PR #165 keeps this intact so maybe the PR merge went wrong?

https://github.com/yjs/y-websocket/commit/882ac9eb79593ef1ecf6c687e97b26865c82878c#diff-bf6cbbe721bbeff64c972b548f6291dc24bba8fd0e1cef5b91d87513c6a0b362L403-L405

jblyberg commented 7 months ago

This is happening for me as well.

Unhandled application error: TypeError: process.on is not a function
WebsocketProvider y-websocket.js:361

Reference:

if (typeof process !== 'undefined') {
  process.on('exit', this._exitHandler)
}
dmonad commented 7 months ago

Note that this only happens because you are using a bundler (or some extension) that tries to polyfill nodejs.

I added an environment check to avoid this. Let me know if this doesn't fix the issue. If not, please supply the specific build steps that cause the issue.

jblyberg commented 7 months ago

This worked for me, thanks. I am using vite 5.0.11. Incidentally, this same issue manifested at the same time in another package I'm using.

kevinvalk commented 6 months ago

@dmonad thanks for adding environment check!

Small note, the check was not added to the destroy method and that can still throw an error in this case (when process is (partially) polyfilled).

https://github.com/yjs/y-websocket/blob/25ee3f0add897a805578beee9e544bfd027e8025/src/y-websocket.js#L402-L404

Can you add the env.isNode to that place as well?