sprockets icon indicating copy to clipboard operation
sprockets copied to clipboard

Better load path warning

Open schneems opened this issue 9 years ago • 2 comments

We need a better way to detect when someone has an asset in their project but isn't referencing it correctly

From https://github.com/rails/rails/issues/23683#issuecomment-211791932

The issue was two fold, first the asset existed but it wasn't in the sprockets path. I think we could add another array of paths that sprockets can track something like "top level asset directory" that we could use to look at all the assets and see if maybe there's a path that wasn't added. We could check this only in development and only when an asset is totally missing.

This creates another class of problems where we didn't define the path where our asset is in inside of a top level directory but this could be pretty easy to detect and convey "Did not find asset "app_icons/evolution.svg" in any folder under top level directory "app/assets".

The second issue was that the asset was referenced incorrectly. Using

<%= image_tag('app_icons/evolution.svg')%>

Instead of

<%= image_tag('evolution.svg')%>

We could explicitly check for this case, where you've included the folder of the asset path and give you a warning "use 'evolution.svg' instead of 'app_icons/evolution.svg`". I don't know how often that happens to justify the special case. I don't know if we could make this a more general case. Either way, I would only check that in development and for failures.

schneems avatar Apr 20 '16 14:04 schneems

Part of the confusion was that the asset helpers fall back to assuming a missing asset is referencing a file in public/, resulting in an /images/… path that obfuscates the underlying issue.

What we're missing is the opportunity to raise and alert the user to exactly what happened and where: We didn't find that asset. Here's where we looked. That's enough to diagnose most cases, so long as we're starting their troubleshooting at the point of failure.

That'd mean splitting the asset helper methods so the main set always expects the referenced asset to exist and a new set, e.g. public_image_tag(…) or image_tag_that_seriously_is_not_an_asset(…), only works with public/ paths, and probably with no existence verification (in case a compile/deploy step provides the assets, or they're proxied or hosted elsewhere).

jeremy avatar Apr 20 '16 17:04 jeremy

I agree, that fallback behavior is hiding a lot.

I like the public_ prefix. I think being more explicit about the behavior you want is a good thing.

schneems avatar Apr 21 '16 16:04 schneems