Cannot augment WebSocket interface in TypeScript

[zimtsui]

when adding a method sendAsync to WebSocket, i have to augment the interface of WebSocket

// promisify.ts
export function promisifySend(socket: WebSocket): void {
    socket.sendAsync = function (/*......*/): Promise<void> {
        /*......*/
    }
}
declare module 'ws' {
    export default interface Websocket { // default export
        sendAsync(/*......*/): Promise<void>;
    }
}

//1.ts
import WebSocket from 'ws'; // import a default export
import { promisifySend } from './promisify.ts';

but this syntax above is illegal in TS, see microsoft/TypeScript#14080

TS only allows to augment a interface which is a named export, for example

// promisify.ts
export function promisifySend(socket: WebSocket): void {
    socket.sendAsync = function (/*......*/): Promise<void> {
        /*......*/
    }
}
declare module 'ws' {
    export interface Websocket { // named export
        sendAsync(/*......*/): Promise<void>;
    }
}

// 1.ts
import { WebSocket } from 'ws'; // import a named export
import { promisifySend } from './promisify.ts';

this requires WebSocket is not default export but a named export.

obviously, this is a bug of TS, not of ws. but you cannot look forward to TS fixing its bug. so i recommend ws exports a WebSocket within WebSocket, like

// index.js
const WebSocket = require('./lib/websocket');
WebSocket.WebSocket = WebSocket;
module.exports = WebSocket;

[lpinca]

We don't ship nor maintain TypeScript type definitions. Please open this issue in the DefinitelyTyped repository.

[zimtsui]

It's required to make a named export in this repository along with TS type definition.

If only TS type definition is modified

import { WebSocket } from 'ws';

will import nothing.

[lpinca]

@zimtsui the same happens if you do

const { WebSocket } = require('ws');

It's not an issue in the library. I don't want to deal with TypeScript limitations/bugs.

[zimtsui]

yes i see, and i said above

obviously, this is a bug of TS, not of ws.

this is only a suggestion. and it needs little code change, may i make a PR?

[lpinca]

No thanks, again, I don't want changes only required for TypeScript.

[zimtsui]

haha OK thank you O(∩_∩)O

[OmgImAlexis]

@zimtsui you can get around this with a hack.

In tsconfig.json I have this "typeRoots": [ "src/@types", "node_modules/@types" ],

This is my src/@types/ws/index.d.ts.

interface WebSocketWithId {
	id: string;
}

declare interface WebSocket extends WebSocketWithId {}
declare namespace WebSocket {
    type id = string;
}

declare module 'ws' {}

When I use import Websocket from 'ws'; I get this.

const ws = new Websocket('http://example.com'); // (parameter) ws: WebSocket
ws.id // (property) WebSocketWithSubscriptions.id: string

[mukaschultze]

Is there some logical reasoning for not wanting to change this or is this simply hate towards Typescript?

[OmgImAlexis]

No thanks, again, I don't want changes only required for TypeScript.

Is there a reason for this? This isn't just a typescript change this would also make intellisense better for standard js users.

[OmgImAlexis]

Also eventually things are moving to ESM this package will need to at some point. This would be a good first change towards that. 😄

[lpinca]

This isn't just a typescript change this would also make intellisense better for standard js users

It is. What the OP requested can easily be done with no changes with CJS and it has nothing to do with intellisense. There is no WebSocket property on the WebSocket object and in my opinion it does not make sense to add it only to work around some TypeScript limitations.

[OmgImAlexis]

This isn't just a typescript change this would also make intellisense better for standard js users

It is. What the OP requested can easily be done with no changes with CJS and it has nothing to do with intellisense. There is no WebSocket property on the WebSocket object and in my opinion it does not make sense to add it only to work around some TypeScript limitations.

Kinda sounds like you’re not aware of how named exports work. 🤔

[OmgImAlexis]

and it has nothing to do with intellisense

I’d suggest you read up on named exports and the reasons to use them. It sounds like you have a personal opinion here that’s clouding your judgement.

Also since node is moving to esm as I said this will eventually need to be moved to it. Negating the cjs comment.

[lpinca]

@OmgImAlexis

1. CJS is not going away anytime soon. There is a whole ecosystem built around it and it will take years before it is abandoned.
2. Who said that WebSocket should be a named export? It will eventually be the default export and not a named export.

[OmgImAlexis]

1. It is as more and more tools are moving to ESM only. I've been running into this more and more over the last few months. I'm sure you'll start to see this as developers start to open more issues here related to it.

2. Making it a named export allows this to work a lot easier to add this type of augmentation on the typescript side. With intellisense it allows the editor to know the name where as default exports don't have that. For example with a named export when I type new Websocket() my editor would know that WebSocket or WS or whatever the var name happens to be and could suggest that as an import. With a default export this can't happen. It's an issue I've found more and more and I've seen a lot of libraries add this for a better developer experience.

[lpinca]

2. Making it a named export allows this to work a lot easier to add this type of augmentation on the typescript side. With intellisense it allows the editor to know the name where as default exports don't have that. For example with a named export when I type new Websocket() my editor would know that WebSocket or WS or whatever the var name happens to be and could suggest that as an import. With a default export this can't happen. It's an issue I've found more and more and I've seen a lot of libraries add this for a better developer experience.

This is not an issue in ws. So in the end, would this change only be useful to work around TypeScript limitations and have some cool IDE suggestions? If so, no thanks.

[OmgImAlexis]

So in the end this change would only be useful to work around TypeScript limitations and have some cool IDE suggestions? If so, no thanks

In the end this wouldn't add any maintenance overhead, nothing would change for users importing it as they current do and people using typescript would get a better experience.

Why exactly are you so against this? There's no downside apart from having to "support" typescript (that's being generous).

[lpinca]

Why exactly are you so against this? There's no downside apart from having to "support" typescript (that's being generous).

It creates a property on the WebSocket object that does not make sense. I don't like that.

[OmgImAlexis]

Why exactly are you so against this? There's no downside apart from having to "support" typescript (that's being generous).

It creates a property on the WebSocket object that does not make sense. I don't like that.

I don’t see why personal opinion should have any relevance here. This would help developers. This wouldn’t add any extra work for you. Why are you holding us back? This doesn’t make any sense.

[OmgImAlexis]

Maybe this is worth taking a vote with contributors of this repo?

[lpinca]

Maybe this is worth taking a vote with contributors of this repo?

Sure. cc: @websockets/core @websockets/contributors

[lpinca]

I don’t see why personal opinion should have any relevance here. This would help developers. This wouldn’t add any extra work for you. Why are you holding us back? This doesn’t make any sense.

Adding that property now is easy. Removing it, if/when the issue is fixed in TypeScript, would be a breaking change... just like WebSocket.createWebSocketStream, WebSocket.Server, WebSocket.Receiver, and WebSocket.Sender. They should not exist.

[OmgImAlexis]

I don’t see why personal opinion should have any relevance here. This would help developers. This wouldn’t add any extra work for you. Why are you holding us back? This doesn’t make any sense.

Adding that property now is easy. Removing it, if/when the issue is fixed in TypeScript, would be a breaking change... just like WebSocket.createWebSocketStream, WebSocket.Server, WebSocket.Receiver, and WebSocket.Sender. They should not exist.

There should be a way to have this work as a named export for esm while keeping the signature as you would like for cjs users. (I hope that makes sense)