jwhitley / requirejs-rails

RequireJS support for your Rails 3 or 4 application
MIT License
592 stars 202 forks source link

Rails 3 compatibility broken #210

Closed peterlong closed 9 years ago

peterlong commented 9 years ago

The commit 913f599cbd436cefee48abfb72866ed621cc7062 adds lib/requirejs/rails/view_proxy.rb which has the line include ActionView::Helpers::AssetUrlHelper in it. This module does not exist in Rails 3. It appears that it is included to get access to javascript_path. The same method is defined in ActionView::Helpers::AssetTagHelper in Rails 3.

Perhaps compatibility could be restored with a code change like this:

module Requirejs
  module Rails
    class ViewProxy
      include ActionView::Context
      if Rails.version.start_with?('4')
        include ActionView::Helpers::AssetUrlHelper 
        include ActionView::Helpers::TagHelper
      else 
        include ActionView::Helpers::AssetTagHelper
      end
    end
  end
end

AssetTagHelper includes TagHelper in Rails 3, so it need not be included again.

froodian commented 9 years ago

At the very least, the gemspec should be updated to state that Rails 4 is required.

dtognazzini commented 9 years ago

How about using semver?

carsomyr commented 9 years ago

All, could you point your Gemfiles to the master branch? If all is well, I'll make a new release.

merqlove commented 9 years ago

0.9.8 still fails at line 82: Rails 3.2.21 Because of sass-rails 3.2.6 javascript_path which has only 1 argument.

  def javascript_path(source, options = {})
    if defined?(super)
      super # ERROR wrong number of arguments (2 for 1)
    else
      view_proxy.javascript_path(source, options)
    end
  end
agis commented 9 years ago

@merqlove I'm curious, have you got the 0.9.9 working on your Rails 3.2 app, or is this issue present in 0.9.9 too? Thanks.

merqlove commented 9 years ago

@agis- Yes, still present. And only my patch fixes it.

agis commented 9 years ago

@merqlove I see, thanks. So you must have run on issues like #247 and this, or? If so, could you share some insight on how you've fixed them.

merqlove commented 9 years ago

@agis- I did pull request. #231

agis commented 9 years ago

@merqlove I'm kinda confused, because your patch in #231 doesn't seem related to #247 and #248.

merqlove commented 9 years ago

@agis- Yes, my patch is for #210 or something.

agis commented 9 years ago

@merqlove Oh ok. My question in the previous comment was referring to whether you've stumbled on #247 and #248 issues too. Anyway, it seems you haven't.

Thanks anyway.

merqlove commented 9 years ago

@agis- Yeah, i have not tried 0.9.9 in production. But in dev it works well.

sun16 commented 8 years ago

Hi everyone,

In addition to the Rails 3.2 compatibility problems mentioned in the above comments, I found a problem in the almond build. A change in the rjs_driver template between 0.9.3 and 0.9.5 caused the almond build to output to almond.js instead of main.js in the tmp directory.

The fix to the above problem was to just use the older working 0.9.3 version.

It would be nice to specify in the readme that 0.9.5 (0.9.3 for Rails 3.2 + and almond) is the last known version with Rails 3 compatibility (possibly a gemspec constraint as well). Or do we intend to continue to support Rails 3.x?

carsomyr commented 8 years ago

@sun16 Almond support is screwed up right now. Does it work normally?

sun16 commented 8 years ago

@carsomyr Sorry about the late response. I have just started using the latest Rails 4.2.5, so I can confirm that almond support is broken regardless of the Rails version. So to update my previous comment, almond support has been broken since 0.9.3, and it's unrelated to Rails 3.x compatibility.

I will create a separate PR for fixing this issue.

carsomyr commented 8 years ago

@sun16 Could you file a separate issue for this?

sun16 commented 8 years ago

Sure, created #255 (have a PR #254 for fixing the almond build)