testdouble / jasmine-rails

A Jasmine runner for rails projects that's got you covered in both the terminal and the browser
http://rubygems.org/gems/jasmine-rails
MIT License
377 stars 154 forks source link

Jasmine includes sprocket files twice. #14

Closed rwz closed 12 years ago

rwz commented 12 years ago

Steps to reproduce:

<head>
    <!--- some html here --->
    <link href="/assets/jasmine.css?body=1" media="screen" rel="stylesheet" type="text/css">
    <script src="/assets/jasmine.js?body=1" type="text/javascript"></script><style type="text/css"></style>
    <script src="/assets/jasmine-html.js?body=1" type="text/javascript"></script>
    <script src="/assets/jquery.js?body=1" type="text/javascript"></script>
    <script src="/assets/jquery_ujs.js?body=1" type="text/javascript"></script>
    <script src="/assets/jquery.js?body=1" type="text/javascript"></script>
    <script src="/assets/jquery_ujs.js?body=1" type="text/javascript"></script>
    <script src="/assets/application.js?body=1" type="text/javascript"></script>
    <!--- some other stuff here --->
</head>

JasmineRails::JhwAdapter.new.js_files returns this:

[
  ".../gems/1.9.1/gems/jasmine-core-1.2.0/lib/jasmine-core/jasmine.js",
  ".../gems/1.9.1/gems/jasmine-core-1.2.0/lib/jasmine-core/jasmine-html.js",
  ".../gems/1.9.1/gems/jquery-rails-2.1.3/vendor/assets/javascripts/jquery.js",
  ".../gems/1.9.1/gems/jquery-rails-2.1.3/vendor/assets/javascripts/jquery_ujs.js",
  ".../my-app/app/assets/javascripts/application.js"
]

Basically, it expands sprocket require calls from application.js file and then include the file itself. Rails javascript_include_tag includes all sprockets requires manually and then tries to include applications.js, sees sprocket require calls in it, expands them and includes jquery and jquery_ujs again.

Expected behavior here would be just stupidly include application.js and let rails handle the rest.

rwz commented 12 years ago

This commit fixed mountable engine behavior for me

https://github.com/rwz/jasmine-rails/commit/9c3a229aed572f58c66aa7e8985a0ba813caa065

searls commented 12 years ago

:+1: Good work! This looks sound and seems reasonable. Next time I'm looking at jasmine-rails I'll try to address this.

rwz commented 12 years ago

I can send a pull request if you wish.

searls commented 12 years ago

That'd be great. I have a feeling that the fix might be even simper if you can work it into the JhwAdapter listing that drives both the JS & CSS file listings

ignu commented 12 years ago

+1 i just lost an hour before finding this issue.