Skip to content

[@types/ws] Remove dependency on node >= 10#26196

Merged
armanio123 merged 1 commit intoDefinitelyTyped:masterfrom
austinried:master
Jun 6, 2018
Merged

[@types/ws] Remove dependency on node >= 10#26196
armanio123 merged 1 commit intoDefinitelyTyped:masterfrom
austinried:master

Conversation

@austinried
Copy link
Copy Markdown
Contributor

@austinried austinried commented Jun 1, 2018

Removes a dependency on net.AddressInfo, which is only present in @types/node@10 and creates a dependency that is not needed. Version 5.x of ws requires node 4.5+ and should not have this dependency: https://github.com/websockets/ws/releases/tag/5.0.0

The error you receive when using types for node versions below 10 is:

node_modules/@types/ws/index.d.ts:179:24 - error TS2694: Namespace '"net"' has no exported member 'AddressInfo'.

Two related issues exist: #25890 (closed) and #26195 (open). The appropriate interface for the return type of the server.address() function is in fact the same as net.AddressInfo | string (docs here: https://github.com/websockets/ws/blob/master/doc/ws.md#serveraddress) so I have extracted the interface and included it in this definition to remove the dependency on node >= 10 types.

  • 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. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jun 1, 2018

@austinried Thank you for submitting this PR!

🔔 @loyd @elithrar @mlamp @TitaneBoy @orblazer - 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.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback Unmerged The author did not merge the PR when it was ready. labels Jun 1, 2018
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jun 6, 2018

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback Unmerged The author did not merge the PR when it was ready. labels Jun 6, 2018
@armanio123 armanio123 merged commit fc9c5c9 into DefinitelyTyped:master Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Owner Approved A listed owner of this package signed off on the pull request. Popular package This PR affects a popular package (as counted by NPM download counts).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants