Skip to content

Conversation

@Aeolun
Copy link

@Aeolun Aeolun commented Jun 18, 2025

This splits out the keys for enabling decompression and compression. I basically wanted to use the library to decompress incoming requests only, but got free compression for my trouble (which caused the issue that I had in the other PR I submitted).

Checklist

@Aeolun Aeolun force-pushed the feat/separate-compression-decompression-flags branch from c95c62f to efba7ef Compare June 18, 2025 04:54
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

Could you run npm run lint:fix?

@Aeolun Aeolun force-pushed the feat/separate-compression-decompression-flags branch from efba7ef to b166462 Compare June 18, 2025 23:42
Co-authored-by: Frazer Smith <[email protected]>
Signed-off-by: Bart Riepe <[email protected]>
@Aeolun
Copy link
Author

Aeolun commented Jun 18, 2025

@Eomm Yeah, hadn't realized I didn't do so until I saw the PR complaining about it. Hopefully good now.

Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

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

lgtm

@Fdawgs Fdawgs requested a review from Copilot July 21, 2025 18:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables separate control of compression and decompression functionality by introducing new configuration options globalCompression and globalDecompression. Previously, the single global option controlled both features together.

  • Adds globalCompression and globalDecompression options that fall back to the existing global option when not specified
  • Updates both compression and decompression parameter processing to respect the new separate flags
  • Documents the new configuration options with usage examples

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
index.js Modifies parameter processing logic to check new global flags before falling back to existing global option
README.md Adds documentation and example for the new globalCompression/globalDecompression configuration
test/global-compress.test.js Adds comprehensive test coverage for the new separate compression/decompression control functionality
Comments suppressed due to low confidence (2)

test/global-compress.test.js:344

  • The zlib module is used but not imported in this file. Add 'const zlib = require('zlib')' at the top of the file.
  test('using gzip when `Accept-Encoding` request header is `gzip`', async (t) => {

test/global-compress.test.js:430

  • The zlib module is used but not imported in this file. Add 'const zlib = require('zlib')' at the top of the file.
    const payload = zlib.gunzipSync(response.rawPayload)

Comment on lines +65 to +67
// only decompress compressed incoming requests
{ globalCompression: false }
)
Copy link

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

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

[nitpick] The example shows only disabling compression but the comment mentions 'only decompress'. Consider showing both options or clarifying that globalDecompression defaults to true.

Suggested change
// only decompress compressed incoming requests
{ globalCompression: false }
)
// Disable compression but keep decompression enabled (default behavior for globalDecompression is true)
{ globalCompression: false }
)
// Disable decompression but keep compression enabled
await fastify.register(
import('@fastify/compress'),
{ globalDecompression: false }
)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants