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.

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

Websocket support for both native Node.js WebSocket and ws. #1975

Open
TarikGul opened this issue Feb 12, 2025 · 3 comments
Open

Websocket support for both native Node.js WebSocket and ws. #1975

TarikGul opened this issue Feb 12, 2025 · 3 comments

Comments

@TarikGul
Copy link
Member

rel: polkadot-js/api#6079

Node.js v21 introduced a native WebSocket, and with v22 its not behind a flag. This causes issues for node because polkadot-js currently grabs the global websocket over the 'ws' lib if it exists. This can be found below.

export const WebSocket = /*#__PURE__*/ extractGlobal('WebSocket', ws);

export function extractGlobal <N extends GlobalNames, T extends GlobalType<N>> (name: N, fallback: unknown): T {
// Not quite sure why this is here - snuck in with TS 4.7.2 with no real idea
// (as of now) as to why this looks like an "any" when we do cast it to a T
//
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return typeof xglobal[name] === 'undefined'
? fallback as T
: xglobal[name] as T;
}

The problem here is that the native WebSocket, and Ws don't have the same ErrorEvent interface which stops the api from succesfully reconnecting. There is a few options here:

  1. Fix the reconnection on the api level, and give support for both interfaces.

  2. Only expose one WebSocket interface in common in this case ws.

@valentinfernandez1
Copy link
Contributor

valentinfernandez1 commented Feb 12, 2025

Alternatively we can force it to use the ws library only on Node versions 22 and above.

const isNode22 = typeof process !== 'undefined' &&
                 process.versions &&
                 process.versions.node &&
                 parseInt(process.versions.node.split('.')[0], 10) >= 22;

export const WebSocket = isNode22 ? ws : /*#__PURE__*/ extractGlobal('WebSocket', ws);

Will give your first suggestion a try and if that doesn't work we can go with this

@TarikGul
Copy link
Member Author

Alternatively we can force it to use the ws library only on Node versions 22 and above.

const isNode22 = typeof process !== 'undefined' &&
process.versions &&
process.versions.node &&
parseInt(process.versions.node.split('.')[0], 10) >= 22;

export const WebSocket = isNode22 ? ws : /#PURE/ extractGlobal('WebSocket', ws);
Will give your first suggestion a try and if that doesn't work we can go with this

I actually like your solution for now - as a patch I think it makes sense! It will atleast keep everything between Apps and Api working perfectly!

@TarikGul
Copy link
Member Author

That being said if we can find the root in the api, that would be the best :)

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

No branches or pull requests

2 participants