close
The Wayback Machine - https://web.archive.org/web/20210112091548/https://github.com/expressjs/compression/pull/158
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

feat: add support for native brotli and iltorb #158

Open
wants to merge 5 commits into
base: master
from

Conversation

@swimmadude66
Copy link

@swimmadude66 swimmadude66 commented Jul 7, 2019

I know there have been a couple pull requests so far to add brotli compression, but it appears they have all stalled due to a lack of support for brotli in zlib < v10.16

This change adds support for native brotli where available, and polyfills that support with iltorb if needed. I added unit tests to ensure this is working and not disrupting any other flows, but let me know if I need to add any more.

Brotli support is disabled by default, but you can opt-in with this option: enableBrotli: true in the top level options object. Additionally, useIltorb was added as a method of testing compatibility between the 2 methods, without requiring environment changes. That option likely will not be terribly useful to an end-user, but rather exists for test cases.

swimmadude66 added 5 commits Jul 7, 2019
updated the README to reflect the new flag to opt in to brotli
restored v10.15 to the travis build list. This is the last LTS version
that does not support brotli in zlib, so it is important to test is, as
well as 10.16 (where brotli was added to zlib)
@waffledonkey
Copy link

@waffledonkey waffledonkey commented Aug 10, 2020

What became of this? It seemed like you were there, just a couple of conflicts but because it's conflicting it looks to have been ignored for over a year while others chase -- potentially -- less elegant solutions. I especially liked your use of iltorb which is currently deprecated in favor of zlib but it was a good idea when you originally did it. :(

@swimmadude66
Copy link
Author

@swimmadude66 swimmadude66 commented Aug 14, 2020

@waffledonkey unfortunately the conflicts were a small part of the problem. Even iltorb requires a higher version of node than the express compression library currently supports. I still would like a compression middleware which supports brotli, but I think the better solution at this point is to create a separate middleware project which operates in a similar way, but does not have to maintain older node compatibility. To that end I started "compress-ts" as a Typescript repo in my own account, but life has gotten in the way of progress on that front lately. I will try and find some time soon to continue that work.

@waffledonkey
Copy link

@waffledonkey waffledonkey commented Aug 14, 2020

Cool thanks. I'll stick with shrink-ray-current and deal with what is being described as a memory leak but I suspect might be cache growth. shrink-ray-current is a 'maintained' (quotes intentional) fork of shrink-ray which itself was based on compression so there might be an easy path forward in there. The one thing that is clever about shrink-ray is that it returns the first request for a thing with light compression and then after the response is sent it maximally compresses and caches the resource for any subsequent requests. How well that works obviously depends on if you burn the first request seeding a CDN, etc. but kinda neat nonetheless. Stay safe

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.