Add support for Brotli compression#326
Conversation
There was a problem hiding this comment.
I like this effort, but the problem is that gopkg.in/kothar/brotli-go.v0/enc comes with dependencies that use Cgo.
We're not likely to merge such dependencies into the github.com/go-chi/chi codebase.
However, if you provided a runtime initialization for additional encoders where you could "inject" the library on-demand, then I'd be down.
ie.
// middleware/compress.go
func AddEncoder(string, func(http.ResponseWriter) http.ResponseWriter)
// example:
init() {
middleware.AddEncoder("br", func(w http.ResponseWriter) http.ResponseWriter {
params := brotli_enc.NewBrotliParams()
params.SetQuality(someLevel)
return brotli_enc.NewBrotliWriter(params, w)
})
}Thoughts?
|
Ah yes, Runtime initialization seems like a good middle way to implement support Brotli until we have a native Go encoder. |
|
Alright, I've added a new patch to support dynamic addition of encoders. Let me know what you think :) |
|
nice work @polyfloyd — could use a review / small iteration but a nice addition. Do you have a separate subpkg somewhere (in another repo) with the Brotli encoder that uses this interface too? if not, we should make one once this interface is settled / merged |
|
@pkieltyka I've added an example in one of the comments on how to set up Brotli compression. |
|
Is this PR waiting on anything before it can be merged? |
|
Hi, I'm back from vacation. I'll try to check out & test this PR in the upcoming week or two. |
|
When this is going to be merged? |
|
Is this PR waiting on anything before it can be merged? I would like to use this feature. Thank you! |
|
I do not believe it is waiting for me! @VojtechVitek? |
|
LGTM |
|
Sorry for delaying this. This is very good work! Thanks @polyfloyd 🎉 LGTM too |
|
I will test in production next week and report back. |
This patch adds support for Google's Brotli compression format currently supported by Chrome, Firefox and IE edge.