Match Dev Asset Experience to Production

[schneems]

Rails is opinionated about how assets should be served in production. In Production assets should be precompiled and then served directly from the public directory. If an asset is not precompiled, than a 404 will be served instead which can render a site useless if an asset is missing.

In development if an asset is NOT flagged to be precompiled (config.assets.precompile), and you reference a file:

<%= asset_path('search.js') %>

Then rails will render the file anyway giving the illusion that the your development code is correct although it will fail in production.

This PR improves the production experience by raising errors in development. When an asset is referenced via asset_path or asset_url an error will be raised in non production environments when the asset is not flagged to be precompiled:

Asset filtered out and will not be served: add `config.assets.precompile += %w( foo.js )` to `config/application.rb` and restart your server

This change only affects the user facing methods, the auto concatenation feature for javascript and CSS will still function correctly in debug mode.

Assets behaving differently in dev and prod is the number one source of invalid support tickets for Heroku. Since the file displays correctly on their machine locally in development, the production problem is incorrectly attributed to a system problem. Once the source of the problem is identified identified it can be difficult to find the exact file with the problem on systems with many assets. This PR will reduce the chances of deploying broken assets to production by increasing visibility into correct behavior in development.

This PR introduces a new config:

# Development
config.assets.raise_asset_errors = true

# Test
config.assets.raise_asset_errors = false

# Production
config.assets.raise_asset_errors = false

[rafaelfranca]

This means we will have to move config.assets.precompile to config/application.rb.

Although I don't have a strong opinion about the assets difference between production and development environments, doesn't make sense to define that config in the development or test environment since nothing is being precompiled.

Also, a lot of behavior is already different between these two environments like the classes are eager loaded and cached and I don't think we need to match these environments.

[robin850]

I love the idea but I'm seeing it very differently. Actually, I would better move this feature to the precompiling rake task. For instance, when running the rake assets:precompile command, I think that we should display something like this:

WARNING: Asset not present: could not find foo.js in any asset directory

Or better exit with a status one ; not only notice the user.

[guilleiguaran]

/cc @josh do you have any opinion about this?

[josh]

I don't think this sort of thing should be implemented at the sprockets-rails level.

Somehow this plugin manages to get more and more bloated everytime I look at it.

[schneems]

Also, a lot of behavior is already different between these two environments like the classes are eager loaded and cached and I don't think we need to match these environments.

In this scenario they are dangerously different, the common case instead of the exception is that a developer will get things working in development but then they will be broken in production without warning.

@robin850 we cannot do it at compile time since the problem comes from usage in the view. Until we render a view we don't know what assets you intend to reference from Rails.

[hone]

So maybe this isn't the right place to do this, but it seems broken to allow users to reference assets that will never be available in production. What does everyone think of the idea that @schneems proposed? If it doesn't belong in sprocket-rails then we can figure something else out.

[wuputah]

As sprockets-rails handles the integration between Rails and Sprockets, this seems the appropriate place to me.

I've definitely been caught by this issue myself as well as seen many others miss this issue. 👍

[robin850]

we cannot do it at compile time since the problem comes from usage in the view. Until we render a view we don't know what assets you intend to reference from Rails.

Wouldn't it be possible to run a headless browser during assets compilation so we can do what your patch propose but not on every request in development ? I'm just asking, I don't know enough of the implementation/internals.

[schneems]

@wuputah thanks for the input, I picked this lib since it required the least amount of dependency injection (the only two internal methods I need are defined in the same file). That being said I'm open to other implementation suggestions.

Are we in agreement that this is a problem worth solving? A quick SO search landed me with quite a few questions around assets not showing up in production and ambiguity about the behavior:

• http://stackoverflow.com/questions/7278029/rails-3-1-asset-precompilation-include-all-javascript-files
• http://stackoverflow.com/questions/10097993/rails-config-assets-precompile-setting-to-process-all-css-and-js-files-in-app-as
• http://stackoverflow.com/questions/8012434/what-is-the-purpose-of-config-assets-precompile
• http://stackoverflow.com/questions/9148310/rails-3-2-1-heroku-asset-precompile-error

Right now we are setting people up for failure when they deploy, let's do something to fix that.

[rafaelfranca]

Are we in agreement that this is a problem worth solving?

👍

[schneems]

To add an extra data point, the second part of this blog post could have been avoided if we have something like this merged in http://viget.com/extend/fixing-missing-assets-with-rails-4-on-heroku.

[brandonweiss]

I agree with @schneems. I have been bitten by this not just once, but several times, because it's so contrary to how I would expect it to work that I actually forget it doesn't already work this way.

[bf4]

👍

[jonatack]

👍 Yes, in my experience this has happened several times to me as well.

[josemarluedke]

👍

[rafaelfranca]

@schneems I have one doubt, if the user put a asset in the public folder, say foo.png and it call <%= asset_path('foo.png') %> what will be the behavior in this PR?

[schneems]

@rafaelfranca I modified this PR to deal with that case: I'm now explicitly testing this case https://github.com/schneems/sprockets-rails/blob/b5d37e6e103d9a6752d5f4691a05eb7743629e13/test/test_helper.rb#L374-L382

If the asset is not found via sprockets, it returns nothing which then triggers these lines https://github.com/rails/rails/blob/master/actionview/lib/action_view/helpers/asset_url_helper.rb#L182-L185 since the asset returned is nil here https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L40-L46
. Long story short, this PR preserves the use case of asset_path and static assets already inside of the public/ folder.

Another thing to note, scoping this behavior to only !Rails.env.production? can cause issues if you're using a javascript library just to test other javascript. Instead I've updated it to use a configurable flag

# Development
config.assets.raise_asset_errors = true

# Test
config.assets.raise_asset_errors = false

# Production
config.assets.raise_asset_errors = false

I'm open to suggestion here.

[guilleiguaran]

I think we can rename config.assets.raise_asset_errors to config.assets.raise_errors since the config variable is already under config.assets namespace 😄

[schneems]

Swapped it for config.assets.raise_runtime_errors to be a bit more specific. After all it's not like setting config.assets.raise_errors = false would prevent all errors associated with assets from being raised.

[lucasmazza]

There is any place where the return value from check_errors_for is relevant? I wonder if just returning nil whenever it doesn't raises an error to be enough.

[lucasmazza]

Given the amount of times I saw @rafaelfranca reminding folks to add an asset to config.assets.precompile I must say that this in indeed a very valuable fix indeed 👍

[rafaelfranca]

Not a fan of sort-circuits ifs. I think this would work:

if assets && assets.send(:matches_filter, precompile || [], source, filename)
  false
else
  true
end

[schneems]

i ususally like them if they let me get rid of a nested if/else or cut down on lines. In this case your suggestion is good. I will do that.

[elia]

👍 🐸

[adammiribyan]

👍

[schneems]

I updated the PR to include code changes minus sstephenson/sprockets#514 which has not been commented on. I also added a CHANGELOG entry, rebased master, and added some happy path tests.

[nfm]

👍 I've stuffed this up a number of times when adding new top-level JS/CSS files.

[mwlang]

+1 -- I need a way to ensure what's in development is good and ready for production and this PR helped me figure out where a bunch of hidden errors were in the *.scss files that were otherwise going uncaught until deployed to production.

[schneems]

Any blockers or additional comments here?

[rafaelfranca]

I was waiting the sprockets change but I think it will not happen soon 😢.

[guilleiguaran]

@schneems thanks for this!!!

[jaredbeck]

Has this been released yet? It looks like there hasn't been a release since 2.0.1 (October 16, 2013). Thanks.

[nfm]

@jaredbeck It's not in a release yet, but it's working on the master branch.