Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop websocket-stream and just use ws #32

Closed
mcollina opened this issue Aug 19, 2019 · 8 comments
Closed

Drop websocket-stream and just use ws #32

mcollina opened this issue Aug 19, 2019 · 8 comments

Comments

@mcollina
Copy link
Member

The ws added support for native node streams. We can drop websocket-stream and just use that.

https://www.npmjs.com/package/ws#use-the-nodejs-streams-api

@Eomm Eomm mentioned this issue Aug 23, 2019
4 tasks
@Eomm
Copy link
Member

Eomm commented Aug 23, 2019

I have started some coding to check it.

Do you know why in the test has been used connection.socket?
It is a matter only of streaming-apis and eventemitter-apis?

@mcollina
Copy link
Member Author

mcollina commented Aug 23, 2019 via email

@Eomm
Copy link
Member

Eomm commented Aug 24, 2019

This is one example:

connection.socket.on('message', message => {
try {
connection.socket.send(message)
} catch (err) {
connection.socket.send(err.message)

@mcollina
Copy link
Member Author

mcollina commented Aug 24, 2019 via email

@fox1t
Copy link
Member

fox1t commented Sep 4, 2019

Hi. That test is correct since it test Should close on unregistered path. Calling it on unregistered path (/) just closes the connection. That's the point of that test.
@Eomm I really don't understand you initial question, can you elaborate?

@jenokizm
Copy link

What is the difference with https://github.com/gj/fastify-ws ?

@mcollina
Copy link
Member Author

What is the difference with https://github.com/gj/fastify-ws ?

This library has many more features.

@Eomm
Copy link
Member

Eomm commented Nov 18, 2019

Closing since this issue has been completed in #38

@Eomm Eomm closed this as completed Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants