Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Incompatibility with NodeJS v22.0.0 #37

Open
NoaHimesaka1873 opened this issue May 14, 2024 · 4 comments · May be fixed by #38
Open

Incompatibility with NodeJS v22.0.0 #37

NoaHimesaka1873 opened this issue May 14, 2024 · 4 comments · May be fixed by #38

Comments

@NoaHimesaka1873
Copy link

NoaHimesaka1873 commented May 14, 2024 •
edited
Loading 

/home/yuifunami/discord-presence-roon/node_modules/@roonlabs/node-roon-api/transport-websocket.js:17
    this.ws.on('pong', () => this.is_alive = true);
            ^

TypeError: this.ws.on is not a function
    at new Transport (/home/yuifunami/discord-presence-roon/node_modules/@roonlabs/node-roon-api/transport-websocket.js:17:13)
    at RoonApi.ws_connect (/home/yuifunami/discord-presence-roon/node_modules/@roonlabs/node-roon-api/lib.js:397:23)
    at Sood.<anonymous> (/home/yuifunami/discord-presence-roon/node_modules/@roonlabs/node-roon-api/lib.js:143:66)
    at Sood.emit (node:events:520:28)
    at Socket.<anonymous> (/home/yuifunami/discord-presence-roon/node_modules/@roonlabs/node-roon-api/sood.js:213:20)
    at Socket.emit (node:events:520:28)
    at UDP.onMessage [as onmessage] (node:dgram:943:8)

Node.js v22.0.0

Arch Linux x86_64 with CachyOS x86_64-v4 packages, NodeJS v22.0.0
@mamsterla
Copy link

I will try to fix and submit a pull request. Ran into this today

@mamsterla
Copy link

I did some work on this and the problem is the poly-fill for browsers now picks up the new WS in nodejs > 22.0. This version of the WS implementation has different signatures. I leave it to the Roon folks to decide whether they want to continue to support Polyfill for web sockets. The workaround is to remove the check for WebSocket defined and always load the ws library. A more complex solution would be changing the transport-websocket class to use the native nodejs one, but there seems to be lack of support for ping/pong and other differences.

@OonihiloO OonihiloO linked a pull request Aug 14, 2024 that will close this issue
Open
@OonihiloO
Copy link

@mamsterla I've submitted a pull request that uses the simplest approach (forcing the use of the polyfill) to this issue.
We'll see if we'll have any kind of reaction from the roon team.

@mamsterla
Copy link

mamsterla commented Aug 14, 2024 via email

@BlueManCZ BlueManCZ mentioned this issue Nov 1, 2024
Open
@joni999 joni999 mentioned this issue Dec 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: add Node.js 22 support OonihiloO/node-roon-api
3 participants
@mamsterla @OonihiloO @NoaHimesaka1873