honojs / hono

Web framework built on Web Standards
https://hono.dev
MIT License
20.05k stars 570 forks source link

`compress` middleware gzips images #3262

Closed jazoom closed 1 month ago

jazoom commented 2 months ago

What version of Hono are you using?

4.5.5

What runtime/platform is your app running on?

Bun

What steps can reproduce the bug?

Use the middleware as intended

app.use(compress())

What is the expected behavior?

Only compress response types that benefit from compression.

What do you see instead?

Images and other non-compressible types are gzip encoded. This wastes CPU for no reason, and often actually results in larger response sizes.

Additional information

No response

jazoom commented 2 months ago

Perhaps the middleware needs something along these lines?

        const contentType = c.res.headers.get("Content-Type");
    if (!contentType) {
        return;
    }

    const isCompressible =
        contentType.includes("text/") ||
        contentType.includes("application/json") ||
        contentType.includes("application/javascript") ||
        contentType.includes("application/xml");

    if (!isCompressible) {
        return;
    }
jazoom commented 2 months ago

It might also be worth having a content size below which compression is not applied.

akku1139 commented 2 months ago

How about specifying the Content-Type for compression in the middleware options?

interface CompressionOptions {
  encoding?: (typeof ENCODING_TYPES)[number],
  contentType?: Array<string>
}
jazoom commented 2 months ago

Cloudflare has a list here which may be a good default

https://developers.cloudflare.com/speed/optimization/content/brotli/content-compression

jazoom commented 2 months ago

Being able to set it as an option also sounds like a great idea.

yusukebe commented 2 months ago

Hi @jazoom @akku1139

This is an interesting issue. We can consider the API and implementation. Anyway, it's an enhancement rather than not a bug. I've changed the label.

yusukebe commented 1 month ago

Now, we can close this issue. Thanks.