search

jwhitley / requirejs-rails

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

Keep assets:precompile compatible with Rails 3.2 #158

Closed lenny closed 10 years ago

lenny commented 10 years ago

ruby_rake_task was being defined locally with a version copied from an older version of Rails. The signature didn't match the one from Rails 3.2 causing the error below.

rake aborted! wrong number of arguments calling ruby_rake_task (2 for 1) pathto/jruby/1.9/gems/actionpack-3.2.17/lib/sprockets/assets.rake:66:in `(root)'

carsomyr commented 10 years ago

@lenny Is it possible to create a ruby_rake_task that has the same signature?

carsomyr commented 10 years ago

Duplicate of #157.

lenny commented 10 years ago

Not sure what fix you'll wind up with. I had imported the most recent ruby_rake_task from Rails 3.2.17. Rails itself invokes ruby_rake_task from multiple places in actionpack/lib/sprockets/assets.rake and should probably pick up it's own version when present. I had originally included the defined? conditional for that purpose. Maybe it would be better to import the method into requirejs-rails_tasks.rake with a different name entirely. e.g. requirejs_ruby_rake_task.

carsomyr commented 10 years ago

How about this? 2298391

lenny commented 10 years ago

The version from Rails would have actually been: def ruby_rake_task(task, fork = true) instead of ruby_rake_task(task, force = true) and would have behaved differently dependent on the value passed in. I think it would be better to either patch in a replacement only if necessary or add in the requirejs_ruby_rake_task for invocation by requirejs-rails without interfering with anything Rails internally had going on. Also think it would be better to use the more recent implementation from Rails 3.2.17.

carsomyr commented 10 years ago

@lenny Link me to the ruby_rake_task source?

lenny commented 10 years ago

https://github.com/rails/rails/blob/3-2-stable/actionpack/lib/sprockets/assets.rake#L4

lenny commented 10 years ago

Actually, looking closer, I see there is a similar issue with invoke_or_reboot, although the implementation of that method hasn't changed since Rails 3.1, but still, why expose requirejs-rails to possible contention? It can be avoided by copying the methods in with a requirejs prefix or patching in only if necessary.

https://github.com/rails/rails/blob/3-2-stable/actionpack/lib/sprockets/assets.rake#L19