-
Notifications
You must be signed in to change notification settings - Fork 105
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
Support for static compression? #104
Comments
That's actually a nice feature. Would you like to PR that? |
I suspect this would need to be implemented by send but it's discouraging that pillarjs/send#108 seems to have stalled out. What is your current feeling on #47? |
At some point I started working on a refactor on send to decouple the generation of the headers from the actual pipe, check out https://github.com/mcollina/send/tree/new-api. It's probably a significant chunk of work, but it would be amazing if you'd like to contribute. |
What is the target node.js of this branch? Your patch doesn't add a readable-stream to package.json as a dependency, I assume Would you mind rebasing your branch against upstream master (it doesn't conflict but just to avoid any change I make causing a conflict). This way I can post PR's to your new-api branch. |
I don’t have time to do any work there unfortunately, I would say take what is there (tests are not passing atm). It’s definitely not finished.. it’s something I coded on a plane. |
Has there been any traction on this? I'd be willing to pitch in if there's something actionable. |
Would you like to send a Pull Request to address this issue? Remember to add unit tests. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I would be interested in contributing to this feature. Is there anything to keep in mind when implementing? The koa-static implementation seems straightforward, but I'm not sure if it would work the same for fastify. Furthermore, what should the API be? I don't think pre-compressed assets are a common use case, but for those that are opting in, I'm thinking it should be possible to skip the |
@chrstntdd sure thing! Let us know how you progress! |
@chrstntdd this is not true for every file. Cases exist where the compressed file would actually be larger than the original which often results in the build process skipping output of that one compressed file. This frequently happens with very small files, especially small image files. Probably could skip checking if the file exists and instead just try opening it with fallback on ENOENT. |
Ah. Thank you mentioning that. If I understand correctly then, the intended behavior would be to attempt to send a pre-compressed version of the asset (if they have opted into the behavior), defaulting to brotli first if we see the accept-encoding headers, falling back to gzip if accepted(?), then falling back to the uncompressed version? |
Yes exactly. |
Not sure if this is the right place to ask but is it possible to support static compressed files: For example if a request is received for
http://localhost/bundle.js
withAccept-Encoding: brotli
, I want to serve the already compressed bundle.js.br instead of compressing bundle.js on the fly. Same ifgzip
compression is requested, I'd want fastify-static to send bundle.js.gz if it exists. For my goal if the pre-compressed file does not exist then I'd want to send the uncompressed file, lack of pre-compressed file would mean that the build system determined that the compressed file was actually larger.Am I correct that for fastify-static to support this it would need to be supported by send? They have a PR to add this feature but it appears to have stalled.
The text was updated successfully, but these errors were encountered: