Document how to enhance assets:precompile to provide compressed assets
#161
Conversation
This seems like useful information to add to the documentation, especially for folks who want to use Propshaft and are running their application behind NGINX with the `gzip_static` configuration. Co-authored-by: Breno Gazzola <[email protected]>
|
Nice one. What do you think about changing the example to use gzip instead of Brotli since it's more supported out of the box by NGINX and other web servers? It could probably be a zero dependency "production ready" solution that uses: https://ruby-doc.org/3.2.0/exts/zlib/Zlib/GzipWriter.html |
|
No strong preference from me. Would you mind using the "Add a suggestion" feature to suggest the specific changes you had in mind using |
|
|
||
| unless compressed_path.exist? | ||
| Propshaft.logger.info "Compressing #{asset.digested_path}" | ||
| `brotli #{asset_path} -o #{compressed_path}` |
There was a problem hiding this comment.
What do you think about using https://ruby-doc.org/3.2.0/exts/zlib/Zlib/GzipWriter.html instead of Brotli since more web servers support GZip out of the box vs Brotli? Then the docs could act as a copy / paste'able production ready example that would work for most folks.
Perhaps the brotli ... line could even remain commented out to show how you could do it for both?
No problem. I can provide confirmation on if it works or not tomorrow morning EST. |
|
Could you move this to the upgrade guide please? Compression is a feature we believe should be handled by something other than propshaft, so this code was just a suggestion for a specific use case and therefore not something we want to put emphasis (which placing on the README does) |
|
I actually considered putting this in the "Upgrading from Sprockets to Propshaft" guide, but I hesitated because even though Sprockets provided this functionality, it's still useful to people who are starting a new application with Propshaft, and those people wouldn't have any reason to look at the "Upgrading from Sprockets to Propshaft" guide. Is there another place besides that guide and the README where this could go? Do we want to add a new document, like |
|
When running this code in production mode with sprockets (propshaft isn't installed), we get this error after the assets get digested: Line 3 is: Steps to reproduce in a repeatable project:
# I've annotated some of the code changes with comments.
# These aren't meant to add to the docs. It's context for why I changed them.
Rake::Task["assets:precompile"].enhance do
assembly = Rails.application.assets
output_path = assembly.config.output_path
assembly.load_path.assets.each do |asset|
asset_path = output_path.join(asset.digested_path)
compressed_path = output_path.join(asset.digested_path.to_s + ".gz")
unless compressed_path.exist?
# I changed this from Propshaft to Rails.
Rails.logger.info "Compressing #{asset.digested_path}"
# Zlib::BEST_COMPRESSION uses 9 as the compression. The default is
# Zlib::DEFAULT_COMPRESSION which is 6. More details can be found at:
# https://docs.ruby-lang.org/en/3.0/Zlib/Deflate.html
Zlib::GzipWriter.open(compressed_path, Zlib::BEST_COMPRESSION) do |gz|
gz.mtime = File.mtime(asset_path)
gz.orig_name = asset_path
gz.write IO.binread(asset_path)
end
end
end
end if Rake::Task.task_defined?("assets:precompile")
The error will happen during the build process. |
@nickjj This code is specific to Propshaft, which does not have a compression feature and should not be used in a project using Sprockets, which has that feature. |
|
@rmacklin How about we take take a page out of Sprockets book and create a "guides" folder? Then we can move the upgrade guide to there too and create an "extending_propshaft.md" file and have this code and explanation there. Could also move the notice about the file watcher that is the readme me to there. And finally edit the readme to point to those guides. |
Right but the code is failing on |
|
If you are asking this just as a curiosity, then You could probably use Rails |
|
I see. I don't want to clutter this PR with unrelated comments but Propshaft's upgrade guide mentions deleting the I'll need to make that customization with Propshaft to be able to test this PR. |
|
If you are using node_modules, you are probably using a bundler like webpack/esbuild. In this cases there's not need to add node_modules to the path. Instead, simply have your bundler place the compiled assets (css/js) into It's pretty much what the bundling gems ( |
|
I'm using esbuild and it's configured to place assets into How can I configure Propshaft to include |
|
Don’t do that. Propshaft paths do not work like Sprockets paths. Anything you add to paths will be copied to the Like I said, have your bundler, which uses |
|
That said, if you really want to do that to see what happens, instead of removing the Rails.application.config.assets.paths.unshift Rails.root.join("node_modules").to_s |
|
Just to make sure I'm 100% following here. Here's what I do today with a combo of jsbundling, cssbundling and sprockets. Everything gets bundled, digested and gzipped as expected and my bundles only include my app's custom JS and CSS:
With Propshaft it sounds like step 1 doesn't change. Step 2 can't change because that's where I Yarn install everything to. The part I'm not clear on is step 3. The upgrade guide says to delete the initializer where this step was previously included but it has no alternative. Maybe this path configuration isn't needed at all with Propshaft and this whole concept goes away? In other words I'm good to go out of the box with my old steps 1 and 2 with Propshaft? |
|
Correct, you don’t need step 3. Propshaft does not care about node modules. It’s esbuild responsibility to read stuff from there when compiling your files. Propshaft only cares about where your compiled files will be places and that’s the builds folder. |
|
Everything mostly worked but when it's set as an initializer I get an error at runtime (more details soon). I also had to make 1 adjustment to the above code snippet. I had to change First the good news. When I added all of the Propshaft changes from the upgrade guide I built my Docker image which performs a It produced this output on disk: The first few bytes of the gzipped files are $ od --format=x1 --read-bytes=10 turbo.min-b405a8972a8538cd316a47ac0fa9b7de2439c484.js.gz
0000000 1f 8b 08 08 4a 4d 44 65 02 03I was also able to successfully However at runtime when the container starts I get this error: hellorails-web-1 | /app/config/initializers/precompile.rb:26:in `<main>': uninitialized constant Rake::Task (NameError)
hellorails-web-1 |
hellorails-web-1 | end if Rake::Task.task_defined?("assets:precompile")
hellorails-web-1 | ^^^^^^
hellorails-web-1 | from /usr/local/bundle/gems/railties-7.1.1/lib/rails/engine.rb:683:in `load'If I remove the if condition then I get the same error but on Technically assets shouldn't be precompiled at runtime but I guess Rails is maybe trying to load this code even if it's not executing it? In which case, any suggestions on how to make this startup properly? |
|
@nickjj Need to change the if in that code I gave you: Rake::Task["assets:precompile"].enhance do
# Code here
end if defined?(Rack) && Rake::Task.task_defined?("assets:precompile") |
|
@brenogazzola It fails with the same type of error as before: |
|
We are working on a new solution to provide compression out of the box without CDN. See the Airlock issue on the Rails 8 milestone. I think that's a better path. |
|
Would it still be possible to provide a working initializer here in case Airlock doesn't solve this use case? Based on the milestone it sounds like it would compress assets on the fly similar to how nginx would compress them, but the original issue for this use case was about pre-compressing them because it's more efficient. The original issue is here btw: #158 |
|
I don’t think that’ll be a concern of high enough importance for most to bother with. Airlock will only compress once per asset, though it will do so on demand. But computers are fast enough that it isn’t a concern I think most people should have to even think about. I want to keep Propshaft as lean as possible. That includes which seeds to plant in people’s minds about what’s needed or not when reading the docs. |
|
Ok thanks, I'll drop a request in the Rails forums to see if we can figure out debugging this almost-working-initializer for folks who plan to stick with using nginx which doesn't only compress assets once. |
|
I understand to not include it in the readme, but maybe it would be helpful to put it in the upgrade guide in section about migrating from sprockets. |
|
I think it'll all be moot because Thruster will be the default in Rails 8, and that'll provide the compression needed. |
#158 provided some good discussion and a code example for how to provide compressed assets. This seems like useful information to add to the documentation, especially for folks who want to use Propshaft and are running their application behind NGINX with the
gzip_staticconfiguration.