[ws]: Add generics to the `ServerOptions` #65501

vansergen · 2023-05-15T18:58:40Z

Changes

Add generics to change the req object type.
[BREAKING CHANGE] Change the default type of the WebSocket generic (used the same approach as [Node]: Update http request listeners #61922 )

class Request extends http.IncomingMessage {}
class MyWebSocket extends WebSocket {}
const server = http.createServer({ IncomingMessage: Request });

Before (`req instanceof http.IncomingMessage`)

new WebSocket.WebSocketServer<MyWebSocket>({
  WebSocket: MyWebSocket,
  server,
}).on("connection", (ws, req) => {
  // $ExpectType MyWebSocket
  ws;
  // $ExpectType http.IncomingMessage
  req;
});

After (no generics at all)

new WebSocket.WebSocketServer({ WebSocket: MyWebSocket, server }).on(
  "connection",
  (ws, req) => {
    // $ExpectType MyWebSocket
    ws;
    // $ExpectType Request
    req;
  }
);

Breaking changes

The following does not seem to work now.

declare module "ws" {
  interface WebSocket {
    id?: string;
  }
}
new WebSocketServer().on("connection", (ws) => {
  // $ExpectType string | undefined
  ws.id;
});

Related PRs

Checklist

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: <>
If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

typescript-bot · 2023-05-15T18:59:20Z

@vansergen Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

ws — on npm, on unpkg

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
🕐 Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 24 days — it is still unreviewed!

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 65501,
  "author": "vansergen",
  "headCommitOid": "c6faa74b2ff857d60964b1483edb5d339032e32d",
  "mergeBaseOid": "06c07f9cdee106a10883ece2d368a244d96722c6",
  "lastPushDate": "2023-05-15T18:35:17.000Z",
  "lastActivityDate": "2023-05-29T20:32:21.000Z",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "ws",
      "kind": "edit",
      "files": [
        {
          "path": "types/ws/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/ws/ws-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "loyd",
        "mlamp",
        "TitaneBoy",
        "reduckted",
        "teidesu",
        "wojtkowiak",
        "k-yle",
        "cwadrupldijjit"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "Knyazhishche",
      "date": "2023-05-29T20:32:21.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 1548400622,
  "ciResult": "pass"
}

typescript-bot · 2023-05-15T18:59:24Z

🔔 @loyd @mlamp @TitaneBoy @reduckted @teidesu @wojtkowiak @k-yle @cwadrupldijjit — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

vansergen · 2023-05-24T18:24:41Z

Did anyone have a chance to take a look at this PR?

typescript-bot · 2023-05-26T18:45:04Z

Re-ping @loyd, @mlamp, @TitaneBoy, @reduckted, @teidesu, @wojtkowiak, @k-yle, @cwadrupldijjit:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

typescript-bot · 2023-06-07T01:06:17Z

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @vansergen.

(Ping @loyd, @mlamp, @TitaneBoy, @reduckted, @teidesu, @wojtkowiak, @k-yle, @cwadrupldijjit.)

JansenMoscol · 2023-06-08T23:13:06Z

With this new code, the package "webpack-dev-server" is broke. I have the follow error

node_modules@types\ws\index.d.ts:328:18 Type 'Server' is not generic.

this will happen with all angular projects that use

vatsalkgor · 2023-06-08T23:49:44Z

Yes same issue here as @JansenMoscol. All our builds are broken as well!

PrashantMohta · 2023-06-09T18:33:30Z

@vansergen seconding this. we have the same issue, was this actually meant to be a "patch" release?

vansergen · 2023-06-09T19:24:24Z

was this actually meant to be a "patch" release?

No, it should be a major release according to semver. I'm not sure how DefinitelyTyped works with breaking PRs. Could you give a snippet to reproduce the error?

PrashantMohta · 2023-06-09T19:31:06Z

was this actually meant to be a "patch" release?

No, it should be a major release according to semver. I'm not sure how DefinitelyTyped works with breaking PRs. Could you give a snippet to reproduce the error?

from what i can gather making an angular project with angular 13 and making a dev build (using ng serve) seems to trigger it because webpack-dev-server uses this.

In case someone finds this PR while trying to solve it for themselves, down-patching by overriding dependencies in package.json works as a workaround.
add the following at the same level as dependencies in your package.json


"overrides": {
    "webpack-dev-server": {
      "@types/ws": "8.5.4"
    }
  },

vansergen · 2023-06-09T19:33:06Z

was this actually meant to be a "patch" release?

No, it should be a major release according to semver. I'm not sure how DefinitelyTyped works with breaking PRs. Could you give a snippet to reproduce the error?

from what i can gather making an angular project with angular 13 and making a dev build (using ng serve) seems to trigger it because webpack-dev-server uses this.

Do you experience the same thing after webpack/webpack-dev-server#4899 is merged

PrashantMohta · 2023-06-09T19:40:38Z

my project uses an older version of angular (@angular-devkit/build-angular@13.3.11) and it's webpack-dev-server seems to be pinned to 4.7.3 so it will probably still be problematic. because webpack-dev-server does not pin @types/ws in the same way.

lyswhut · 2023-06-10T15:49:23Z

Same problem, this breaks extensions to socket:

import type WS from 'ws'
import { WebSocketServer } from 'ws'
interface Socket extends WS.WebSocket {
  // ...
}
const socketServer: WS.Server<Socket> = new WebSocketServer({
  noServer: true,
})

vansergen · 2023-06-10T15:53:14Z

@lyswhut Now, there is no need to use generics. Try the following

import { WebSocket, WebSocketServer } from "ws";
class Socket extends WebSocket {
  // ...
}
const socketServer = new WebSocketServer({ noServer: true, WebSocket: Socket });

lyswhut · 2023-06-10T16:06:38Z

For some reason I only extended the type of socket, is there a way to make this work?

import { type WebSocket, WebSocketServer } from 'ws'
interface Socket extends WebSocket {
  // ...
}
const socketServer = new WebSocketServer<Socket>({
  noServer: true,
})

Or mark it as breaking change? It does break some apps.

vansergen · 2023-06-11T17:56:48Z

is there a way to make this work?

Yes.

Change interface to class
Pass the Socket to the constructor

lyswhut · 2023-06-12T04:15:06Z

The first option would mean changing a lot of existing code.

Pass the Socket to the constructor

@vansergen I tried this but it doesn't work:

import { WebSocket, WebSocketServer } from 'ws'
interface Socket extends WebSocket {
  id?: string
  // ...
}
const socketServer = new WebSocketServer({
  noServer: true,
  WebSocket: WebSocket as unknown as Socket,
})

socketServer.on('connection', (socket, request) => {
  console.log(socket.id)
})

matozoid · 2023-06-12T08:42:40Z

This broke our angular project too. Please revert this.

tahsinpol · 2023-06-13T15:08:58Z

This broke our angular project too. Please revert this.

I agree. We can't build

vansergen · 2023-06-13T15:24:27Z

This broke our angular project too. Please revert this.

Could you provide an example? I explained above how to adopt to the latest version.

masterqwerty · 2023-06-13T20:06:33Z

@vansergen For the angular builds, I don't believe it's a problem with the codebase. With the angular builds, the problem is in a nested dependency of @angular-devkit/build-angular, webpack-dev-server, which depends on @types/ws. In our projects, webpack-dev-server has the version constraint ^8.2.2 for @types/ws, which pulls version 8.5.5, which is a breaking change. So without an override config in the package.json, the angular build breaks with the following error:

The solution here would be to move version 8.5.5 to 9.0.0, as it's a major change and not a patch. Or at least create version 8.5.6 that reverts the change, and create a new 9.0.0 based off of 8.5.5. That way, newer projects can pull in version 9.0.0 and use the generics, and it doesn't break older projects that aren't ready to migrate.

vansergen · 2023-06-13T20:23:13Z

the problem is in a nested dependency of @angular-devkit/build-angular, webpack-dev-server

The problem has been fixed in the webpack-dev-server@4.15.1

vansergen · 2023-06-13T20:35:56Z

@masterqwerty It seems that @angular-devkit/build-angular uses the fixed version webpack-dev-server@4.15.0

    "webpack-dev-server": "4.15.0",

A new PR should be created to bump it.

masterqwerty · 2023-06-13T20:43:33Z

The problem has been fixed in the webpack-dev-server@4.15.1

In our project we don't control the version of webpack-dev-server. The version of webpack-dev-server that gets pulled by @angular-devkit/build-angular is 4.7.3.

It seems that @angular-devkit/build-angular uses the fixed version

We're using an older version of Angular, and upgrading to the fixed version would require a major version bump to Angular, and we're not in a position to migrate just yet.

tahsinpol · 2023-06-13T20:52:11Z

I found a temporary solution. I added the following code to the package.json file.

"overrides": {
"webpack-dev-server": {
"@types/ws": "8.5.4"
}
}

Node.js version >= v16.14.0. should be. See: https://ruleoftech.com/2022/override-nested-npm-dependency-versions

This is not a permanent solution.

Also, those who use angular old version may not be able to update node.js version.

vatsalkgor · 2023-06-13T22:05:19Z

@tahsinpol I agree. Even after my PR this will not fix the issue in older angular version and needs to move to a new 8.5.6 version (with reverted changes) to make it work with the older angular versions. @vansergen Can you please start the work to revert the PR? Adding "overrides" should not be a solution used by thousands of people using older angular versions.

* fix(ws): add generics to the server options * test(ws): add generics to `WebSocket` and `IncomingMesage`

vansergen added 2 commits May 15, 2023 21:26

fix(ws): add generics to the server options

dd169e3

test(ws): add generics to WebSocket and IncomingMesage

c6faa74

typescript-bot added the Critical package label May 15, 2023

typescript-bot added this to Waiting for Code Reviews in New Pull Request Status Board May 15, 2023

typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board May 15, 2023

typescript-bot added the Unreviewed No one showed up to review this PR, so it'll be reviewed by a DT maintainer. label May 26, 2023

Knyazhishche approved these changes May 29, 2023

View reviewed changes

typescript-bot added the Other Approved This PR was reviewed and signed-off by a community member. label May 29, 2023

typescript-bot moved this from Needs Maintainer Review to Needs Maintainer Action in New Pull Request Status Board Jun 7, 2023

weswigham merged commit b068334 into DefinitelyTyped:master Jun 8, 2023
2 checks passed

typescript-bot removed this from Needs Maintainer Action in New Pull Request Status Board Jun 8, 2023

vatsalkgor mentioned this pull request Jun 9, 2023

chore(deps): bump up @types/ws package webpack/webpack-dev-server#4899

Merged

6 tasks

vansergen deleted the ws/http branch June 9, 2023 06:32

aesteves60 mentioned this pull request Jun 20, 2023

fix: force @types/ws version ovh/design-system#76

Merged

masterqwerty mentioned this pull request Jun 21, 2023

[ws] Reverts generics added to ServerOptions for @types/ws v8 #65849

Closed

8 tasks

Desplandis pushed a commit to Desplandis/DefinitelyTyped that referenced this pull request Jul 3, 2023

[ws]: Add generics to the ServerOptions (DefinitelyTyped#65501)

841c61c

* fix(ws): add generics to the server options * test(ws): add generics to `WebSocket` and `IncomingMesage`

Muhammad-Altabba mentioned this pull request Jul 24, 2023

Error: node_modules/@types/ws/index.d.ts:328:18 - error TS2315: Type 'Server' is not generic. web3/web3.js#6303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ws]: Add generics to the `ServerOptions` #65501

[ws]: Add generics to the `ServerOptions` #65501

vansergen commented May 15, 2023 •

edited

typescript-bot commented May 15, 2023 •

edited

typescript-bot commented May 15, 2023

vansergen commented May 24, 2023

typescript-bot commented May 26, 2023

typescript-bot commented Jun 7, 2023

JansenMoscol commented Jun 8, 2023

vatsalkgor commented Jun 8, 2023

PrashantMohta commented Jun 9, 2023

vansergen commented Jun 9, 2023 •

edited

PrashantMohta commented Jun 9, 2023 •

edited

vansergen commented Jun 9, 2023 •

edited

PrashantMohta commented Jun 9, 2023

lyswhut commented Jun 10, 2023

vansergen commented Jun 10, 2023

lyswhut commented Jun 10, 2023 •

edited

vansergen commented Jun 11, 2023 •

edited

lyswhut commented Jun 12, 2023

matozoid commented Jun 12, 2023

tahsinpol commented Jun 13, 2023

vansergen commented Jun 13, 2023

masterqwerty commented Jun 13, 2023

vansergen commented Jun 13, 2023 •

edited

vansergen commented Jun 13, 2023 •

edited

masterqwerty commented Jun 13, 2023

tahsinpol commented Jun 13, 2023 •

edited

vatsalkgor commented Jun 13, 2023 •

edited

[ws]: Add generics to the ServerOptions #65501

[ws]: Add generics to the ServerOptions #65501

Conversation

vansergen commented May 15, 2023 • edited

Changes

Before (req instanceof http.IncomingMessage)

After (no generics at all)

Breaking changes

Related PRs

Checklist

typescript-bot commented May 15, 2023 • edited

1 package in this PR

Code Reviews

Status

Inactive

typescript-bot commented May 15, 2023

vansergen commented May 24, 2023

typescript-bot commented May 26, 2023

typescript-bot commented Jun 7, 2023

JansenMoscol commented Jun 8, 2023

vatsalkgor commented Jun 8, 2023

PrashantMohta commented Jun 9, 2023

vansergen commented Jun 9, 2023 • edited

PrashantMohta commented Jun 9, 2023 • edited

vansergen commented Jun 9, 2023 • edited

PrashantMohta commented Jun 9, 2023

lyswhut commented Jun 10, 2023

vansergen commented Jun 10, 2023

lyswhut commented Jun 10, 2023 • edited

vansergen commented Jun 11, 2023 • edited

lyswhut commented Jun 12, 2023

matozoid commented Jun 12, 2023

tahsinpol commented Jun 13, 2023

vansergen commented Jun 13, 2023

masterqwerty commented Jun 13, 2023

vansergen commented Jun 13, 2023 • edited

vansergen commented Jun 13, 2023 • edited

masterqwerty commented Jun 13, 2023

tahsinpol commented Jun 13, 2023 • edited

vatsalkgor commented Jun 13, 2023 • edited

[ws]: Add generics to the `ServerOptions` #65501

[ws]: Add generics to the `ServerOptions` #65501

vansergen commented May 15, 2023 •

edited

Before (`req instanceof http.IncomingMessage`)

typescript-bot commented May 15, 2023 •

edited

vansergen commented Jun 9, 2023 •

edited

PrashantMohta commented Jun 9, 2023 •

edited

vansergen commented Jun 9, 2023 •

edited

lyswhut commented Jun 10, 2023 •

edited

vansergen commented Jun 11, 2023 •

edited

vansergen commented Jun 13, 2023 •

edited

vansergen commented Jun 13, 2023 •

edited

tahsinpol commented Jun 13, 2023 •

edited

vatsalkgor commented Jun 13, 2023 •

edited