Permalink
Browse files

Merge pull request #8756 from causes/js_include_tag_fix

Fix javascript_include_tag when no js runtime is available
  • Loading branch information...
2 parents 002dfba + 9bc5e65 commit 686307cd90cd2153f73bd60a3810411584d3f6d3 @guilleiguaran guilleiguaran committed Jan 10, 2013
View
@@ -6,6 +6,11 @@
* Do not append second slash to `root_url` when using `trailing_slash: true`
+* Prevent unnecessary asset compilation when using javascript_include_tag on
+ files with non-standard extensions.
+
+ *Noah Silas*
+
* Do not append second slash to root_url when using `trailing_slash: true`
Fix #8700.
@@ -157,18 +157,25 @@ def rewrite_asset_path(source, dir, options = {})
end
def rewrite_extension(source, dir, ext)
- source_ext = File.extname(source)
- if ext && source_ext != ".#{ext}"
- if !source_ext.empty? && (asset = asset_environment[source]) &&
- asset.pathname.to_s =~ /#{source}\Z/
- source
- else
- "#{source}.#{ext}"
- end
- else
+ source_ext = File.extname(source)[1..-1]
+
+ if !ext || ext == source_ext
+ source
+ elsif source_ext.blank?
+ "#{source}.#{ext}"
+ elsif exact_match_present?(source)
source
+ else
+ "#{source}.#{ext}"
end
end
+
+ def exact_match_present?(source)
+ pathname = asset_environment.resolve(source)
@keithpitt

keithpitt Mar 4, 2013

Why use asset_environment.resolve(source) here instead of asset = asset_environment[source]?

@keithpitt

keithpitt Mar 4, 2013

Never mine me. Silly question.

@umate

umate Apr 8, 2013

@guilleiguaran this line produces a lot of exceptions on our project (~200 coffee files, ~100 sass) in development mode (and in assets precompiling on production).
Each rising Sprockets::FileNotFound exception uses 4Mb of memory. Also each pathname lookup takes over 5Mb.
The rails application with so many assets uses over 2Gb RAM for now. This is really bad
I've replaced asset_environment.resolve(source) with asset_environment[source] and it returned back to ~400Mb.
It's pretty critical for big applications I think. Any suggestions here?

+ pathname.to_s =~ /#{Regexp.escape(source)}\Z/
+ rescue Sprockets::FileNotFound
+ false
+ end
end
end
end
@@ -270,6 +270,13 @@ def compute_host(source, request, options = {})
javascript_include_tag(:application, :extra)
end
+ test "precompiled assets with an extension when no JS runtime is available" do
+ @config.assets.compile = false
+ @config.assets.digests = {'foo.min.js' => 'foo.min-f00.js'}
+ Sprockets::Index.any_instance.stubs(:build_asset).raises
+ assert_nothing_raised { javascript_include_tag('foo.min') }
+ end
+
test "stylesheet path through asset_path" do
assert_match %r{/assets/application-[0-9a-f]+.css}, asset_path(:application, :ext => "css")

0 comments on commit 686307c

Please sign in to comment.