[TS] Change websocket library in Node #2989
Conversation
|
We'll need to do the needful after this is merged. Or wait, it's semi-automatic now? |
| (val instanceof ArrayBuffer || | ||
|
|
||
| // Sometimes we get an ArrayBuffer that comes from a polyfill | ||
| (val.constructor && val.constructor.name === "ArrayBuffer")); |
There was a problem hiding this comment.
Why is the formatting so bad?
|
This is different runtime dependency, so it still needs @Eilon to approve |
| export function isArrayBuffer(val: any): val is ArrayBuffer { | ||
| return val && typeof ArrayBuffer !== "undefined" && | ||
| (val instanceof ArrayBuffer || | ||
| // Sometimes we get an ArrayBuffer that comes from a polyfill |
There was a problem hiding this comment.
nit: I wrote this comment when I was randomly speculating :). Just say something like "Sometimes we get an ArrayBuffer that doesn't satisfy instanceof 🤷♂️ "
| "nan": "^2.3.3", | ||
| "typedarray-to-buffer": "^3.1.2", | ||
| "yaeti": "^0.0.6" | ||
| "async-limiter": "~1.0.0" |
There was a problem hiding this comment.
Fewer extra deps. Nice :)
| @@ -32,7 +32,7 @@ | |||
| "karma-summary-reporter": "^1.5.0", | |||
| "ts-node": "^4.1.0", | |||
| "typescript": "^3.0.1", | |||
| "websocket": " ^1.0.26" | |||
| "ws": " ^6.0.0" | |||
There was a problem hiding this comment.
So I need to get approval for this one?
There was a problem hiding this comment.
Yes, we are swapping in a pure-JavaScript websocket library instead of the previous WebSocket library we were using which had a native component.
|
@BrennanConroy removals don't matter. |
#2973
Also, #2986
Inverted a couple
instanceof ArrayBufferchecks because theArrayBufferreturned fromwsisn't an ArrayBuffer or something super odd, so checking for string instead and assuming buffer otherwise works.See nodejs/node#20978 for some context maybe? @anurse maybe you'll understand it better.