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

Detect when compression middleware is installed? #72

Open
rexxars opened this issue Feb 10, 2016 · 4 comments
Open

Detect when compression middleware is installed? #72

rexxars opened this issue Feb 10, 2016 · 4 comments
Assignees
Labels

Comments

@rexxars
Copy link

rexxars commented Feb 10, 2016

I am the author of a library called sse-channel, which implements server-sent events (SSE). For this to work in an efficient and fast manner, I need to be able to call flush on responses that pass through this compression middleware.

Currently I'm just checking for the presence of a flush method on the response and calling that, but since node has this method, it's causing me some issues. Is there a way I can detect whether the flush method is from this middleware, or if the compression middleware is applied to the response, somehow?

@dougwilson
Copy link
Contributor

No, there isn't a good way to know if this module is active or not (yet?), but perhaps you may just not want your SSE responses compressed at all? If that is the case, then your code can always set the header Cache-Control: no-transform (https://github.com/expressjs/compression#compressionoptions) and this module won't even compress the response, and so doesn't require the users to setup anything special and you don't have to call .flush().

Let me know if you really do want your responses to be compressed, and I'm sure we can think of a good way to provide the indication to your code :)

@dougwilson dougwilson self-assigned this Feb 10, 2016
@rexxars
Copy link
Author

rexxars commented Feb 10, 2016

I'm unsure of how much impact gzip will have on SSE, since we're explicitly telling it to flush before having basically any data to work with. If gzip can somehow keep optimizing data as it comes in even though the "initial chunk" is sent, then it might actually be able to provide a pretty significant improvement. Any idea if this is the case or not?

@dougwilson
Copy link
Contributor

Any idea if this is the case or not?

Yes, this is the case. Your initial chunks between .flush calls are probably not going to be very efficient (probably will be even larger than the text content itself), but if you keep the connection open long and have a lot of repetitive data, I believe the dictionary is built up between the chunks (but I would have to check).

So it sounds like the take away may be to determine if the dictionary is actually shared between the chunks or not, so it can be better evaluated if the compression is actually helping or hurting the stream (I believe it does help). In parallel, we can probably think of a good way to indicate if the response is getting compressed in a unique way that is obvious and doesn't conflict too much (darn Node.js taking res.flush() ;) ).

gigabo added a commit to gigabo/compression that referenced this issue Feb 17, 2016
Supports use such as:

```javascript
// Not going to write again for a while.  If compression middleware is
// installed let's tell it to flush what we've got through to the client.
if (res.flushCompression) {
    res.flushCompression();
}
```

Currently the `res` object has a `flush` method regardless of whether
compression middleware is installed.  The built-in `flush` method issues a
noisy deprecation warning.  This patch provides an alias for the `flush`
method to allow detection of the compression middleware's `flush`.

This addresses expressjs#72.
@gigabo gigabo mentioned this issue Feb 17, 2016
gigabo added a commit to gigabo/compression that referenced this issue Jun 27, 2016
Supports use such as:

```javascript
// Not going to write again for a while.  If compression middleware is
// installed let's tell it to flush what we've got through to the client.
if (res.flushCompression) {
    res.flushCompression();
}
```

Currently the `res` object has a `flush` method regardless of whether
compression middleware is installed.  The built-in `flush` method issues a
noisy deprecation warning.  This patch provides an alias for the `flush`
method to allow detection of the compression middleware's `flush`.

This addresses expressjs#72.
@rexxars
Copy link
Author

rexxars commented Jul 4, 2016

#74 would solve this for me. Any feedback on that, @dougwilson ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants