search

jwhitley / requirejs-rails

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

Use config.assests.js_compressor setting for save original js compressor #206

Open aar0nTw opened 9 years ago

aar0nTw commented 9 years ago

When I set config.assets.js_compressor to compressor with some custom options like Uglifier.new(mangle: false)
The precompile task always get the error: Sprockets::Error: unknown compressor: js_compressor, there's because Sprocket::Environment will return :js_compressor symbol when we want to get Rails.application.assets.js_compressor value if user set compressor to specific compressor object , please see https://github.com/sstephenson/sprockets/blob/master/lib/sprockets/compressing.rb#L60

and when the task want to put orignal_js_compressor back to env.js_compressor, Sprocket can't find any compressor source name :js_compressor and throw error.

So it should be cache the Rails.application.config.assets.js_compressor(A real compressor object) to original_js_compressor not Rails.application.assets.js_compressor.

ref #201

carsomyr commented 9 years ago

@aar0nTw Could you add a js_compressor attribute to Requirejs::Rails::Config and use that?

aar0nTw commented 9 years ago

Hi @carsomyr:

It already has optimize option for r.js build in build_config_whitelist, we can use optimize: uglify2 and set more option for detail build output in requirejs.yml.

I think it has no need to add attribute in requirejs rails config, We just need get the current sprocket compressor object, because the prepare_source task is call sprockets to get assets path and write it to requirejs source path for r.js build, so it temporarily disable the compress function of sprockets and restore it after the task end.

carsomyr commented 9 years ago

@aar0nTw My main concern is clarity: The code in the Rake tasks refers to requirejs.config, so an effort should be made to adhere to that.

aar0nTw commented 9 years ago

@carsomyr updated, please check it

agis commented 9 years ago

Can we get this merged?

carsomyr commented 9 years ago

@agis- Ok, I'll take a look at this again.

agis commented 8 years ago

Run into the same issue too, using requirejs-rails 1.9.5, rails 4.2, sprockets 2.12.4 and sprockets-rails.

@aar0nTw Are you using this patch in production?

Dagnan commented 8 years ago

Also interested in this (requirejs-rails-0.9.9 / rails 4.2).