Skip to content

refactor: remove obsolete brotli and http2 support checks #221

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.

Sign up for GitHub

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

Merged
bjohansebas merged 2 commits into from
Apr 7, 2025
Merged

refactor: remove obsolete brotli and http2 support checks #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d92f23
refactor: remove obsolete brotli and http2 support checks
Phillip9587 Feb 14, 2025
374e699
fix(docs): remove note about brotli support
Phillip9587 Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ The following compression codings are supported:
- gzip
- br (brotli)

**Note** Brotli is supported only since Node.js versions v11.7.0 and v10.16.0.

## Install

This is a [Node.js](https://nodejs.org/en/) module available through the
Expand Down
26 changes: 8 additions & 18 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,13 @@ var zlib = require('zlib')
module.exports = compression
module.exports.filter = shouldCompress

/**
* @const
* whether current node version has brotli support
*/
var hasBrotliSupport = 'createBrotliCompress' in zlib

/**
* Module variables.
* @private
*/
var cacheControlNoTransformRegExp = /(?:^|,)\s*?no-transform\s*?(?:,|$)/
var SUPPORTED_ENCODING = hasBrotliSupport ? ['br', 'gzip', 'deflate', 'identity'] : ['gzip', 'deflate', 'identity']
var PREFERRED_ENCODING = hasBrotliSupport ? ['br', 'gzip'] : ['gzip']
var SUPPORTED_ENCODING = ['br', 'gzip', 'deflate', 'identity']
var PREFERRED_ENCODING = ['br', 'gzip']

var encodingSupported = ['gzip', 'deflate', 'identity', 'br']

Expand All @@ -56,16 +50,12 @@ var encodingSupported = ['gzip', 'deflate', 'identity', 'br']

function compression (options) {
var opts = options || {}
var optsBrotli = {}

if (hasBrotliSupport) {
Object.assign(optsBrotli, opts.brotli)

var brotliParams = {}
brotliParams[zlib.constants.BROTLI_PARAM_QUALITY] = 4

// set the default level to a reasonable value with balanced speed/ratio
optsBrotli.params = Object.assign(brotliParams, optsBrotli.params)
var optsBrotli = {
...opts.brotli,
params: {
[zlib.constants.BROTLI_PARAM_QUALITY]: 4, // set the default level to a reasonable value with balanced speed/ratio
...opts.brotli?.params
}
}

// options
Expand Down
47 changes: 14 additions & 33 deletions test/compression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,10 @@ var crypto = require('crypto')
var http = require('http')
var request = require('supertest')
var zlib = require('zlib')

var describeHttp2 = describe.skip
try {
var http2 = require('http2')
describeHttp2 = describe
} catch (err) {
if (err) {
console.log('http2 tests disabled.')
}
}
var http2 = require('http2')

var compression = require('..')

/**
* @const
* whether current node version has brotli support
*/
var hasBrotliSupport = 'createBrotliCompress' in zlib
var brotli = hasBrotliSupport ? it : it.skip

describe('compression()', function () {
it('should skip HEAD', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
Expand Down Expand Up @@ -322,7 +306,7 @@ describe('compression()', function () {
.expect(200, done)
})

describeHttp2('http2', function () {
describe('http2', function () {
it('should work with http2 server', function (done) {
var server = createHttp2Server({ threshold: 0 }, function (req, res) {
res.setHeader(http2.constants.HTTP2_HEADER_CONTENT_TYPE, 'text/plain')
Expand Down Expand Up @@ -518,7 +502,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: br"', function () {
brotli('should respond with br', function (done) {
it('should respond with br', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -532,7 +516,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: br" and passing compression level', function () {
brotli('should respond with br', function (done) {
it('should respond with br', function (done) {
var params = {}
params[zlib.constants.BROTLI_PARAM_QUALITY] = 11

Expand All @@ -547,7 +531,7 @@ describe('compression()', function () {
.expect('Content-Encoding', 'br', done)
})

brotli('shouldn\'t break compression when gzip is requested', function (done) {
it('shouldn\'t break compression when gzip is requested', function (done) {
var params = {}
params[zlib.constants.BROTLI_PARAM_QUALITY] = 8

Expand Down Expand Up @@ -592,8 +576,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: gzip, br"', function () {
var brotli = hasBrotliSupport ? it : it.skip
brotli('should respond with br', function (done) {
it('should respond with br', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -605,9 +588,7 @@ describe('compression()', function () {
.expect('Content-Encoding', 'br', done)
})

brotli = hasBrotliSupport ? it.skip : it

brotli('should respond with gzip', function (done) {
it.skip('should respond with gzip', function (done) {
Copy link
Member Author

@Phillip9587 Phillip9587 Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set this test to it.skip because we may need it since there is an open issue for it: #220

var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -621,7 +602,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: deflate, gzip, br"', function () {
brotli('should respond with br', function (done) {
it('should respond with br', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -635,7 +616,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: gzip;q=1, br;q=0.3"', function () {
brotli('should respond with gzip', function (done) {
it('should respond with gzip', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -649,7 +630,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: gzip, br;q=0.8"', function () {
brotli('should respond with gzip', function (done) {
it('should respond with gzip', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -663,7 +644,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: gzip;q=0.001"', function () {
brotli('should respond with gzip', function (done) {
it('should respond with gzip', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand All @@ -677,7 +658,7 @@ describe('compression()', function () {
})

describe('when "Accept-Encoding: deflate, br"', function () {
brotli('should respond with br', function (done) {
it('should respond with br', function (done) {
var server = createServer({ threshold: 0 }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand Down Expand Up @@ -828,7 +809,7 @@ describe('compression()', function () {
.end()
})

brotli('should flush small chunks for brotli', function (done) {
it('should flush small chunks for brotli', function (done) {
var chunks = 0
var next
var server = createServer({ threshold: 0 }, function (req, res) {
Expand Down Expand Up @@ -934,7 +915,7 @@ describe('compression()', function () {
.expect(200, 'hello, world', done)
})

brotli('should compress when enforceEncoding is brotli', function (done) {
it('should compress when enforceEncoding is brotli', function (done) {
var server = createServer({ threshold: 0, enforceEncoding: 'br' }, function (req, res) {
res.setHeader('Content-Type', 'text/plain')
res.end('hello, world')
Expand Down