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

feature: Allow serving compressed SVG images (rebased) #50359

Merged
merged 1 commit into from Dec 14, 2023

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented Dec 14, 2023

Motivation / Background

This adds image/svg+xml to the compressible content types of ActionDispatch::Static

(This is a rebased re-submission of #42407 by @ledermann which was approved by @zzak in 2021)

Detail

Original description:

This adds image/svg+xml to the compressible content types of ActionDispatch::Static so GZIP or Brotli can be used.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionpack label Dec 14, 2023
This adds `image/svg+xml` to the compressible content types
of ActionDispatch::Static

Co-authored-by: Mike Dalessio <mike.dalessio@gmail.com>
@flavorjones flavorjones force-pushed the serve-svg-with-compression_rebase branch from b4c21b9 to 0fc5b06 Compare December 14, 2023 16:38
@byroot byroot merged commit b9cc0a2 into rails:main Dec 14, 2023
4 checks passed
@flavorjones flavorjones deleted the serve-svg-with-compression_rebase branch December 14, 2023 16:56
@tvdeyen
Copy link
Contributor

tvdeyen commented Mar 3, 2024

@byroot could Rails core team consider to release this in the next patch level of 7.1 please 🙏🏻?

Since the footprint is very small, but the impact on apps huge and it already has been approved in 2021 (but never merged) it would be great to not have to wait until 7.2 (or 8) comes out.

/cc @dhh @rafaelfranca

@skipkayhil
Copy link
Member

could Rails core team consider to release this in the next patch level of 7.1 please 🙏🏻?

(Not on the Core team, so take my opinion with a grain of salt)

I don't see a reason to backport a feature like this when you can add this to your app today:

config.public_file_server.enabled = false # disable the default Static middleware configuration
config.middleware.insert_after ::Rack::Sendfile, ActionDispatch::Static, config.paths["public"].first, index: config.public_file_server.index_name, headers: config.public_file_server.headers || {}, compressible_content_types: /\A(?:text\/|application\/javascript|image\/svg\+xml)/

@tvdeyen
Copy link
Contributor

tvdeyen commented Mar 3, 2024

Nice. Will try. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants