Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Ensure assets aren't duplicated for debug. #31 #33

Merged
merged 1 commit into from

4 participants

@jejacks0n

When debug=true assets can require the same file multiple times -- this ensures that even if several files are included using javascript_include_tag only one dependency file will ever be included (even if multiple files require it).

@jejacks0n

Travis failed:

Bundler could not find compatible versions for gem "sprockets":
In Gemfile:
   sprockets-rails (>= 0) ruby depends on
     sprockets (~> 2.8) ruby
   actionpack (~> 3.2.0) ruby depends on
     sprockets (2.2.2)
@jejacks0n

Adding rails/rails#8735 for reference. A separate fix for rails 3-2-stable that addresses the same issue.

@guilleiguaran guilleiguaran commented on the diff
lib/sprockets/rails/helper.rb
@@ -95,7 +95,7 @@ def javascript_include_tag(*sources)
else
super(source, options)
end
- }.join("\n").html_safe
+ }.flatten.uniq.join("\n").html_safe
@guilleiguaran Owner

we need the flatten?

If you're asking me and not starting a discussion.. Yes, without it it's an array of arrays -- thus uniq is semi-ineffective. Check the test case without the flatten and you'll see the dependency twice.

Oh wait -- not positive -- will double check tomorrow.

@guilleiguaran Owner

right, thanks for clarification :smile:

Yup, removing flatten causes the test cases to fail (with the dependency included twice).. it's the same logic as what needs to be in 3-2-stable in rails/rails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilleiguaran guilleiguaran merged commit 0991710 into rails:master

1 check failed

Details default The Travis build failed
@josh
Collaborator

Makes sense.

@jejacks0n jejacks0n referenced this pull request in modeset/teaspoon
Closed

Is my SpecController running slow? #57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 2, 2013
  1. @jejacks0n
This page is out of date. Refresh to see the latest.
View
4 lib/sprockets/rails/helper.rb
@@ -95,7 +95,7 @@ def javascript_include_tag(*sources)
else
super(source, options)
end
- }.join("\n").html_safe
+ }.flatten.uniq.join("\n").html_safe
@guilleiguaran Owner

we need the flatten?

If you're asking me and not starting a discussion.. Yes, without it it's an array of arrays -- thus uniq is semi-ineffective. Check the test case without the flatten and you'll see the dependency twice.

Oh wait -- not positive -- will double check tomorrow.

@guilleiguaran Owner

right, thanks for clarification :smile:

Yup, removing flatten causes the test cases to fail (with the dependency included twice).. it's the same logic as what needs to be in 3-2-stable in rails/rails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
else
sources.push(options)
super(*sources)
@@ -117,7 +117,7 @@ def stylesheet_link_tag(*sources)
else
super(source, options)
end
- }.join("\n").html_safe
+ }.flatten.uniq.join("\n").html_safe
else
sources.push(options)
super(*sources)
View
1  test/fixtures/dependency.css
@@ -0,0 +1 @@
+/* dependency */
View
1  test/fixtures/dependency.js
@@ -0,0 +1 @@
+// dependency
View
2  test/fixtures/file1.css
@@ -0,0 +1,2 @@
+/*= require dependency
+ */
View
1  test/fixtures/file1.js
@@ -0,0 +1 @@
+//= require dependency
View
2  test/fixtures/file2.css
@@ -0,0 +1,2 @@
+/*= require dependency
+ */
View
1  test/fixtures/file2.js
@@ -0,0 +1 @@
+//= require dependency
View
4 test/test_helper.rb
@@ -296,6 +296,8 @@ def test_javascript_include_tag
@view.javascript_include_tag(:foo)
assert_equal %(<script src="/assets/foo.js?body=1"></script>\n<script src="/assets/bar.js?body=1"></script>),
@view.javascript_include_tag(:bar)
+ assert_equal %(<script src="/assets/dependency.js?body=1"></script>\n<script src="/assets/file1.js?body=1"></script>\n<script src="/assets/file2.js?body=1"></script>),
+ @view.javascript_include_tag(:file1, :file2)
end
def test_stylesheet_link_tag
@@ -305,6 +307,8 @@ def test_stylesheet_link_tag
@view.stylesheet_link_tag(:foo)
assert_equal %(<link href="/assets/foo.css?body=1" media="screen" rel="stylesheet" />\n<link href="/assets/bar.css?body=1" media="screen" rel="stylesheet" />),
@view.stylesheet_link_tag(:bar)
+ assert_equal %(<link href="/assets/dependency.css?body=1" media="screen" rel="stylesheet" />\n<link href="/assets/file1.css?body=1" media="screen" rel="stylesheet" />\n<link href="/assets/file2.css?body=1" media="screen" rel="stylesheet" />),
+ @view.stylesheet_link_tag(:file1, :file2)
end
def test_javascript_path
Something went wrong with that request. Please try again.