Registering assets for precompilation from within gem not working.

[jejacks0n]

Hi, I'm the author of Teaspoon, a javascript test runner for rails, and a new issue showed up recently around some of the assets not being available in "production". Teaspoon doesn't use the production env, so that part is a bit confusing, and it does register its assets for precompilation.

It would appear at a glance that between sprockets v2* and v3 we're getting some breakage that's not fully clear. Hopefully someone here can shed some light as to why, so we can patch it up.

Thanks!

https://github.com/modeset/teaspoon/issues/443#issuecomment-166356580

[rafaelfranca]

Cross posting my answer here:

@jejacks0n hi! I'm sprockets maintainer. What is happening is that teaspoon use javascript_tag to include the users' defined test files. Say you have the test/javascript/unit/home_test.js file. Teaspoon call javascript_tag with that as argument. That file is not registered for precompilation and sprockets-rails 3 now is strict about files that are being used as argument to javascript_tag and not being in the precompile list.

Does teaspoon maintain a list of all the test files? If so it is possible to register them?

[jejacks0n]

Thanks, I'll close this one and we can work out the details in the other issue. =)

[jejacks0n]

@rafaelfranca I found what appears to be a fix, but I wanted to run it past you first.

so, in https://github.com/rails/rails/blob/master/actionview/lib/action_view/helpers/asset_tag_helper.rb#L58

this passes path_options through to path_to_javascript/javascript_path -- and inside sprockets_rails, I see this logic that allows allow_non_precompiled as an argument:

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L274

To get things working again in teaspoon, the single line change is actually in asset_tag_helper.rb to basically allow propagating that allow_non_precompiled logic through -- otherwise I don't see a usefulness to it, but could be missing some unrelated behavior that could utilize the allow_non_precompiled argument.

I added this in asset_tag_helper.rb#L59:

        path_options[:debug] = options['allow_non_precompiled']

This allows specifying allow_non_precompiled: true in the javascript_include_tag options, and from an API consumer in the way of Teaspoon, that seems like the least likely to break.

[jejacks0n]

The commit, and line that's being proposed as an addition to the public API can be found:

https://github.com/modeset/teaspoon/commit/1e3a22311fc505caf70e20f752a59e19a2eab9d2#diff-217b7bb2bfb27a38e0efc7e62600e23eR105

I'm happy to submit a PR to action_view if you feel this is an acceptable and reasonable solution for projects like Teaspoon.

[jejacks0n]

ping. =)

[rafaelfranca]

@jejacks0n I like that plan, but I have a doubt about teaspoon implementation. Why are you passing debug and allow_non_precompiled? I can't see where debug is being used.

[jejacks0n]

Thanks!

You'll see at https://github.com/rails/rails/blob/master/actionview/lib/action_view/helpers/asset_tag_helper.rb#L58 that some options are pulled out and passed to path_to_javascript. if :debug is present in that options hash it makes it down to https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L274 as the third argument which is otherwise inaccessible as far as I can tell. It could be an oversight, or incomplete implementation?

It's the only means for which to get the third argument in sprockets-rails asset_path to be true, which will allow non precompiled assets when you explicitly opt in to allow it (as is needed in our case). The feature is essentially already there, just unavailable.

However it's implemented is up to you, and you can do it, or I can jump in and follow your suggested implementation as a PR. I've contributed a few times to Rails in similar areas, so whatever. I just figured it would get less lost as an issue here before I opened a PR on rails/rails.

[rafaelfranca]

I mean, you call is javascript_include_tag 'foo', debug: true, allow_non_precompiled: true, why passing debug if it is not being used at all?

[jejacks0n]

Haha, that's a good question, but they seem to be intended for different things. See, :debug at the top level seems to be intentionally excluded from the hash passed to the path_to_javascript method because it's used further down the chain to determine if assets should be combined or if they should be served separately. If I were to just pass :debug through to path_to_javascript, the behavior of debug: true would be to 1. not combine files, 2. allow files not matched by the precompile rules. That may not be the logic desired.

[jejacks0n]

If it is the logic you see being desired, that's what we can try. I'd have to dig in further to determine how asset configuration in different environments may impact how that behaves.

[jejacks0n]

And to answer why I pass debug: true to the javascript_include_tag helper. That's to expand assets so stack traces in errors are more useful -- path_to_implementation_file.js:25 is more useful than application.js:1067

[rafaelfranca]

I may be missing something obvious but the only place that I see debug being used is in path_to_javascript. You patch seems to set debug to the same value that allow_non_precompiled before sending to path_to_javascript, so would not that means that you don't need to pass debug at all?

[jejacks0n]

I do, it makes it elsewhere in the chain to indicate that we want assets expanded -- unless I'm mistaken on how javascript_include_tag(foo, debug: true) behaves. What I've always believed is that it adds the body=true param when requesting the assets. (that param may have changed recently, but it behaves the same)

[rafaelfranca]

Yes, it does that, but is is done inside path_to_javascript so if you set path_options[:debug] = options['allow_non_precompiled'] before calling path_to_javascript no matter the debug value you pass to javascript_include_tag what really matter is the value of allow_non_precompiled so javascript_include_tag(foo, debug: true, allow_non_precompiled: true) is the same as javascript_include_tag(foo, debug: false, allow_non_precompiled: true)

[jejacks0n]

I think you may be mistaken. That's not the case as I read it here: https://github.com/rails/rails/blob/master/actionview/lib/action_view/helpers/asset_tag_helper.rb#L58

path_options = options.extract!('protocol', 'extname', 'host').symbolize_keys

'debug' is not included in the extracted options that are passed to path_to_javascript. Is that the bug? If so, debug: true will mean to expand the assets and allow non precompiled assets. That's fine, I just want to make sure.

[rafaelfranca]

Yeah, that code is confusing the actual implementation of javascript_include_tag that we are looking is https://github.com/rails/sprockets-rails/blob/a5cc186302b299d2f5904c7b13f12aaf7ac7f30d/lib/sprockets/rails/helper.rb#L123-L147. I'll take a look to see how we can implement that but your plan seems fine to me.

[jejacks0n]

Hmm.. haha, I'll do some debugging this evening and check back on this. I'm curious now how in my stack traces I never ended up in the one that's in sprockets-rails, and that the quick hack did change the behavior if we're not actively going through that code path. Need a better understanding. =)

Thanks for your time helping sort this out. Much appreciated.

[rafaelfranca]

My pleasure! I'm curious now too, hahahah. If you have an example project that you use to debug and could share it would be awesome

[liseki]

Hi @rafaelfranca. I am helping to fix some asset issues in Teaspoon which are related to this thread of thought. It seems that here https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/rails/helper.rb#L71-L79 we have the beginning of a confusion that conflates the meanings of the 'debug' and 'allow_non_precompiled' options; the 'debug' option is passed along to resolve_asset_path where its meaning turns into 'allow_non_precompiled'. Why not pass the 'debug' option to legacy_debug_path and the 'allow_non_precompiled' option to resolve_asset_path thus keeping their meanings separate? If this makes sense to you would you accept such a patch?

[mathieujobin]

5 years later, I am reading this and wonder if we could remove the monkey patch from teaspoon