Conversation
222563b to
8696a9d
Compare
lib/websocket-server.js
Outdated
|
|
||
| /** @typedef {import('http').Server} Server */ | ||
| /** @typedef {import('http').IncomingMessage} IncomingMessage */ | ||
| /** @typedef {import('net').Socket} Socket */ |
There was a problem hiding this comment.
if you already require('http') then you don't need typedef?
lib/websocket.js
Outdated
| * @property {Number} CONNECTING The connection is not yet open | ||
| * @property {Number} OPEN The connection is open and ready to communicate | ||
| * @property {Number} CLOSING The connection is in the process of closing | ||
| * @property {Number} CLOSED The connection is closed |
There was a problem hiding this comment.
tried this also, didn't work for me, maybe it's the right way to do it...
what did work was:
class X {
static CONNECTING = 1
CONNECTING = 1
}or like a mention earlier: x.CONNECTING = 1
but then it dose not have the right descriptor (unless you add those with defineProperties)
There was a problem hiding this comment.
Does it work with
Object.defineProperty(WebSocket.prototype, 'CONNECTING', { enumerable: true, value: 0 });
Object.defineProperty(WebSocket, 'CONNECTING', { enumerable: true, value: 0 });(without the loop)?
There was a problem hiding this comment.
The same pattern is also used here https://github.com/websockets/ws/blob/7.5.2/lib/websocket.js#L402-L411. I'm not sure I want to change it. It is not a JSDoc issue. It is simply not documented/commented :)
There was a problem hiding this comment.
without the loop it did work fine doing just
Object.defineProperty(WebSocket.prototype, 'CONNECTING', { enumerable: true, value: 0 });tried it if it would work with a object using defineProperties
Object.defineProperties(WebSocket, {
CONNECTING: {
enumerable: true,
value: 0
}
});The result: Property 'CONNECTING' does not exist on type 'typeof WebSocket'
|
@jimmywarting PTAL. |
|
LGTM |
|
Thank you for the review. |
Refs: #1910