[PATCH] Allow custom javascript/stylesheet expansion symbols

Cool idea. My only problem with it is that it slightly changes the order of :defaults.

Previously:

expand_javascript_sources :defaults, 'lowpro'
#=> ['prototype', 'effects', 'dragdrop', 'controls', 'lowpro', 'application']

Now:

expand_javascript_sources :defaults, 'lowpro'
#=> ['prototype', 'effects', 'dragdrop', 'controls', 'application', 'lowpro']

This will break for people who were expecting application.js to be included after everything else (people like me).

keep application.js at the end

Thanks for the input Brandon. I updated the patch to keep application.js at the end.

Looks good to me now.

+1

This tastes more like a plugin to me.

Why is that eigentone? One of the main purposes for this change is to give plugins an unobtrusive hook into the core framework.

How is this supposed to be tested? I applied your diff, created a monkey.js file and added to my layout

<% javascript_include_tag :monkey %>

And I get the symbols error.

railsjitsu:

You would want to do something like this...

# environment.rb
ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey, "head", "body", "tail"

# create public/javascripts/head.js
# create public/javascripts/body.js
# create public/javascripts/tail.js

# application.html.erb
<% javascript_include_tag :monkey %>

You should see that this puts in javascript tags for "head", "body" and "tail"

As stated in the description of report {68}, the 2.0 milestone should only be used by core members for real show stoppers.

+1 for me as well.

This enhances functionality that is already in core, that is why I think it should be committed. I can already include my own javascript files, but it's tedious when there is a common set of scripts I always use. This is not adding superfluous functionality into core, it's merely adding more value to a feature that already exists.

I can see where it would be useful to build a plugin on top of this which makes the register_javascript_expansion call for you.

+1

Looks like a good idea. But the patch is lacking documentation.

Looks good guys, just a few thoughts though:

1) are there any plugins which need this functionality? What do their authors think? 2) If we're going to make symbols work outside of :defaults, it'd be nice to raise a nice exception for :some_random_symbol.

Please add "verified" keyword after adding documentation as suggested here. I reckon javascript_include_tag and stylesheet_link_tag are good places to add the relevant documentation/examples.

Thanks.

Added documentation

Two points :

1. Why can't it get rid of JAVASCRIPT_DEFAULT_SOURCES constant ? 2. As Koz said, it should raise proper exception for :some_random_symbol

Looks good apart from that. One more thing though,

ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => ["head", "body", "tail"]

ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => ["head", "body", "tail"], :funky => ["head2", "tail2"]

May be the above format is a bit more intuitive and allows more than registering one expansions with a single call.

The only reason I left JAVASCRIPT_DEFAULT_SOURCES, register_javascript_include_default, and reset_javascript_include_default was for backwards compatibility. If we are comfortable with breaking that, I can get rid of one or all of those things.

Changed register method to take a hash, raise exception on unknown symbol

(In [9016]) Added ActionView::Helpers::register_javascript/stylesheet_expansion to make it easier for plugin developers to inject multiple assets (closes #10350) [lotswholetime]

This breaks asset caching pretty hardcore since it doesn't keep the default sources first. Prototype is included alphabetically in the cache, breaking anything that precedes & relies on it.

I don't understand how this was verified, guys. These +1 are way out of hand.

(In [9063]) Revert [9106]. References #10350.

I am not experiencing the same problem you are. I have this in my view...

<%= javascript_include_tag :defaults, :dhtml_calendar, :cache => "combined" %>

My combined.js file is in the proper order.

What is the scenario you have where this breaks?

javascript_include_tag :all

Thanks for the bug report bitsweat. I have updated the patch to include a test for the expected behavior of keeping the defaults first (this was untested before), and updated the implementation accordingly.

Fixed problem with :all expansion not keeping the default sources at the start

Thank you!

(In [9065]) Re-added ActionView::Helpers::register_javascript/stylesheet_expansion to make it easier for plugin developers to inject multiple assets. Closes #10350.

(In [9066]) Missed adds. References #10350.