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

refactor: change default brotli compression level to 4 #1

Merged

Conversation

Not-Jayden
Copy link

A default compression level of 11 is likely too extreme/slow for most use cases.

Cloudflare recommends a default of 4 https://blog.cloudflare.com/this-is-brotli-from-origin/#brotli-compression-to-11

the fastify-compress plugin changed their default based on this recommendation too fastify/fastify-compress#287 (comment)

export const ZLIB_LEVEL = 6 as const
export const THRESHOLD_SIZE = 1024
export const ZSTD_LEVEL = 2
export const BROTLI_LEVEL = 4
Copy link
Author

Choose a reason for hiding this comment

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

Actual change, just removed the as const's as they're redundant here on primitives.

Copy link
Owner

Choose a reason for hiding this comment

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

Nice catch!

@Not-Jayden
Copy link
Author

Side note, were issues intentionally disabled in this repo? I intended to open this as a discussion first before making the change given it's ultimately an opinionated default (e.g. node seems to have 11 as their default).

@Not-Jayden Not-Jayden marked this pull request as ready for review November 5, 2024 03:10
@vuolter
Copy link
Owner

vuolter commented Nov 6, 2024

Side note, were issues intentionally disabled in this repo? I intended to open this as a discussion first before making the change given it's ultimately an opinionated default (e.g. node seems to have 11 as their default).

I didn't realise the issues were disabled, I've turned them on now.
Thanks for reporting and for this PR ofc 😃

@vuolter vuolter merged commit 50df48b into vuolter:main Nov 6, 2024
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.

2 participants