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

Fix(SprocketsRenderer) support Sprockets::Manifest if assets were precompiled #430

Merged
merged 5 commits into from
Feb 4, 2016

Conversation

rmosolgo
Copy link
Member

Previously, SprocketsRenderer would live-compile assets, even in production. However, this is impossible in Rails 5 (not just improper) because there is no Sprockets::Environment at Rails.application.assets.

Instead, there is a Sprockets::Manifest at Rails.application.assets_manifest which tells where each file's compiled counterpart is.

So, if the Sprockets::Environment isn't available, use the manifest.

TODO

  • Prefer the Manifest over the Environment (to avoid double-compiling assets)
  • Somehow, handle cases where assets are pushed to a CDN, not the local Rails server
    • Check for asset_host config?, no because that doesn't necessarily mean the files are remote
    • Extract get_asset_content to classes
    • Accept a custom AssetContainer

@losvedir
Copy link

Somehow, handle cases where assets are pushed to a CDN, not the local Rails server

I think in most cases, doesn't the CDN just serve essentially as a caching layer, with the underlying Rails server still serving the assets (the first time)?

At least in our setup with heroku/cloudfront, which I think is fairly common, we precompile assets so they're served via the /public/assets/ folder, and we use, e.g. <%= stylesheet_link_tag 'app' %>. We have our asset_host set to our cloudfront CDN, so the generated link ends up looking like https://abc123.cloudfront.net/assets/app-def345.css.

Then when the first request comes in to cloudfront after a new deploy, the filename hash will have changed, so cloudfront will proxy through to our site, and then cache and serve the asset from then on.

  • Check for asset_host config?

This wouldn't exactly work for us, I don't think. We do have it set, but we still have the assets precompiled on the server as well, so maybe accessible to Sprockets? (I'm not 100% clear how sprockets works...).

Just FYI.

@rmosolgo
Copy link
Member Author

Yeah, I think I was off the mark on asset_host. It sounds like your case would be handled by the default Sprockets::Manifest.

I was just poking around our own production environment, though, and I noticed that our assets aren't in public/assets, so I'll have to figure out how to handle that!

@rmosolgo
Copy link
Member Author

Ug, looks like Manifest has changed a lot in the different sprockets versions

rmosolgo pushed a commit that referenced this pull request Feb 4, 2016
Fix(SprocketsRenderer) support Sprockets::Manifest if assets were precompiled
@rmosolgo rmosolgo merged commit 6d44757 into master Feb 4, 2016
@rmosolgo rmosolgo deleted the support-asset-manifest branch February 4, 2016 15:54
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.

None yet

3 participants