search

rexxars / sse-channel

Server-Sent Events "channel" where all messages are broadcasted to all connected clients, history is maintained automatically and server attempts to keep clients alive by sending "keep-alive" packets automatically.
MIT License
111 stars 11 forks source link

Replace deprecated flush() with flushHeaders() #7

Closed wKovacs64 closed 8 years ago

wKovacs64 commented 8 years ago

Node was complaining about the use of the now-deprecated flush() function:

OutgoingMessage.flush is deprecated. Use flushHeaders instead.

Replacing flush() with flushHeaders() removes the warning.

rexxars commented 8 years ago

Hrm. Thanks for the PR, but I don't think this does what we expect it to. The flush is only actually meant to be called when using the compression-middleware for express (or similar), as that triggers the gzip stream to be flushed. This PR will actually remove this functionality in favor of preventing some deprecation warnings.

Not 100% sure how to solve this. The best solution would be something that only triggered flush() if the response is a compressed one, otherwise don't call flush/flushHeaders at all.

wKovacs64 commented 8 years ago

Yeah I was uncertain of this as well. Although it sounded like they just added flushHeaders and deprecated flush rather than renaming flush so I assume they do the same thing. Perhaps not?

rexxars commented 8 years ago

It's the same on plain Node responses, however the compression middleware monkey-patches a new flush method on top of the response which calls flush on the underlying gzip stream. It's confusing.

wKovacs64 commented 8 years ago

I see what you mean. Closing this for now, further research required.