Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

deps: add ws types to main dependencies #246

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

maschad
Copy link
Member

@maschad maschad commented May 29, 2023

While following the example of how to do a basic libp2p setup, if we only configure webSockets as our transport the types will be undefined.

node_modules/@libp2p/websockets/dist/src/index.d.ts:5:36 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

5 import type { ClientOptions } from 'ws';
                                     ~~~~

Perhaps these types should be exported in a better way? Especially since they are required in it-ws but if so then maybe the are not properly exported there

node_modules/it-ws/dist/src/client.d.ts:1:36 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import type { ClientOptions } from 'ws';
                                     ~~~~

node_modules/it-ws/dist/src/duplex.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ws'.

1 /// <reference types="ws" />
                        ~~

node_modules/it-ws/dist/src/sink.d.ts:1:32 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import type { WebSocket } from 'ws';
                                 ~~~~

node_modules/it-ws/dist/src/web-socket.d.ts:1:23 - error TS7016: Could not find a declaration file for module 'ws'. '/Users/horizon/Desktop/test/node_modules/ws/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/ws` if it exists or add a new declaration (.d.ts) file containing `declare module 'ws';`

1 import WebSocket from 'ws';
                        ~~~~

@maschad maschad requested a review from achingbrain May 29, 2023 17:02
@achingbrain
Copy link
Member

Perhaps these types should be exported in a better way? Especially since they are required in it-ws but if so then maybe the are not properly exported there

Agree they should be moved to deps for it-ws but we have a dependency on ws here too so we should depend on the types as well, otherwise there's no guarantee that it-ws will ship with the types for the version of ws we expect.

@achingbrain achingbrain merged commit 628e260 into libp2p:master Jun 6, 2023
github-actions bot pushed a commit that referenced this pull request Jun 6, 2023
## [6.0.2](v6.0.1...v6.0.2) (2023-06-06)

### Dependencies

* add ws types to main dependencies ([#246](#246)) ([628e260](628e260)), closes [/github.com/alanshaw/it-ws/blob/master/package.json#L50](https://github.com/libp2p//github.com/alanshaw/it-ws/blob/master/package.json/issues/L50)
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

🎉 This PR is included in version 6.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@maschad
Copy link
Member Author

maschad commented Jun 6, 2023

Perhaps these types should be exported in a better way? Especially since they are required in it-ws but if so then maybe the are not properly exported there

Agree they should be moved to deps for it-ws but we have a dependency on ws here too so we should depend on the types as well, otherwise there's no guarantee that it-ws will ship with the types for the version of ws we expect.

Cool, Opened a PR alanshaw/it-ws#71 @achingbrain

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants