Merge pull request #7276 from sikachu/3-2-stable-js-include-tag-fix

Do not include application.js if it doesn't exists
rails · Aug 6, 2012 · 3cc872d · 3cc872d
2 parents 0fb6bbd + 6c46730
commit 3cc872d
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 7 deletions.
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Rails 3.2.8 ##
 
+*   `javascript_include_tag :all` will now not include `application.js` if the file does not exists. *Prem Sichanugrist*
+
 *   Reverted the deprecation of `:confirm`. *Rafael Mendonça França*
 
 *   Reverted the deprecation of `:disable_with`. *Rafael Mendonça França*

diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb
@@ -27,7 +27,8 @@ def custom_dir
 
           def expand_sources(sources, recursive = false)
             if sources.include?(:all)
-              all_asset_files = (collect_asset_files(custom_dir, ('**' if recursive), "*.#{extension}") - ['application']) << 'application'
+              all_asset_files = (collect_asset_files(custom_dir, ('**' if recursive), "*.#{extension}") - ['application'])
+              add_application_js(all_asset_files, sources)
               ((determine_source(:defaults, expansions).dup & all_asset_files) + all_asset_files).uniq
             else
               expanded_sources = sources.inject([]) do |list, source|
@@ -40,7 +41,7 @@ def expand_sources(sources, recursive = false)
           end
 
           def add_application_js(expanded_sources, sources)
-            if sources.include?(:defaults) && File.exist?(File.join(custom_dir, "application.#{extension}"))
+            if (sources.include?(:defaults) || sources.include?(:all)) && File.exist?(File.join(custom_dir, "application.#{extension}"))
               expanded_sources.delete('application')
               expanded_sources << "application"
             end
@@ -101,8 +102,8 @@ def javascript_path(source)
         #
         #   config.action_view.javascript_expansions[:defaults] = %w(foo.js bar.js)
         #
-        # When using <tt>:defaults</tt>, if an <tt>application.js</tt> file exists in
-        # <tt>public/javascripts</tt> it will be included as well at the end.
+        # When using <tt>:defaults</tt> or <tt>:all</tt>, if an <tt>application.js</tt> file exists
+        # in <tt>public/javascripts</tt> it will be included as well at the end.
         #
         # You can modify the HTML attributes of the script tag by passing a hash as the
         # last argument.
@@ -129,16 +130,16 @@ def javascript_path(source)
         #   #    <script type="text/javascript" src="/javascripts/rails.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/application.js?1284139606"></script>
         #
-        # * = The application.js file is only referenced if it exists
+        # Note: The application.js file is only referenced if it exists
         #
         # You can also include all JavaScripts in the +javascripts+ directory using <tt>:all</tt> as the source:
         #
         #   javascript_include_tag :all
         #   # => <script type="text/javascript" src="/javascripts/jquery.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/rails.js?1284139606"></script>
-        #   #    <script type="text/javascript" src="/javascripts/application.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/shop.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/checkout.js?1284139606"></script>
+        #   #    <script type="text/javascript" src="/javascripts/application.js?1284139606"></script>
         #
         # Note that your defaults of choice will be included first, so they will be available to all subsequently
         # included files.
@@ -161,9 +162,9 @@ def javascript_path(source)
         #   javascript_include_tag :all, :cache => true
         #   # => <script type="text/javascript" src="/javascripts/jquery.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/rails.js?1284139606"></script>
-        #   #    <script type="text/javascript" src="/javascripts/application.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/shop.js?1284139606"></script>
         #   #    <script type="text/javascript" src="/javascripts/checkout.js?1284139606"></script>
+        #   #    <script type="text/javascript" src="/javascripts/application.js?1284139606"></script>
         #
         #   # assuming config.perform_caching is true
         #   javascript_include_tag :all, :cache => true

diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
@@ -338,6 +338,15 @@ def test_reset_javascript_expansions
     assert_raise(ArgumentError) { javascript_include_tag(:defaults) }
   end
 
+  def test_all_javascript_expansion_not_include_application_js_if_not_exists
+    FileUtils.mv(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.js'),
+      File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.bak'))
+    assert_no_match(/application\.js/, javascript_include_tag(:all))
+  ensure
+    FileUtils.mv(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.bak'),
+      File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.js'))
+  end
+
   def test_stylesheet_path
     ENV["RAILS_ASSET_ID"] = ""
     StylePathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) }