Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Move config.assets out into sprockets-rails plugin #7981

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Member

josh commented Oct 18, 2012

Alright @rafaelfranca, got another thing.

It seems to work pretty good if the Sprockets Railtie just defines its own config.assets as an options set.

https://github.com/rails/sprockets-rails/blob/master/lib/sprockets/railtie.rb#L48-55

@josh josh commented on the diff Oct 18, 2012

railties/lib/rails/engine.rb
@@ -588,9 +588,11 @@ def load_seed
end
initializer :append_assets_path, group: :all do |app|
- app.config.assets.paths.unshift(*paths["vendor/assets"].existent_directories)
- app.config.assets.paths.unshift(*paths["lib/assets"].existent_directories)
- app.config.assets.paths.unshift(*paths["app/assets"].existent_directories)
+ if app.config.respond_to?(:assets)
+ app.config.assets.paths.unshift(*paths["vendor/assets"].existent_directories)
+ app.config.assets.paths.unshift(*paths["lib/assets"].existent_directories)
+ app.config.assets.paths.unshift(*paths["app/assets"].existent_directories)
+ end
@josh

josh Oct 18, 2012

Member

But I really hate doing this. How can we move this out too?

@drogus

drogus Oct 18, 2012

Member

Can't we just add railtie to sprockets-rails and create an initializer there?

@rafaelfranca

rafaelfranca Oct 18, 2012

Owner

@drogus in this case not. This is a initializer that have to run in every engine. I think we can move this to sprockets-rails, reopen Rails::Engine there and add this initializer, since the sprockets/railtie is loaded before the engines.

Contributor

josevalim commented Oct 18, 2012

I don't think we should completely remove the config.assets from Rails. For example, config.assets.paths can be consumed by other applications (I believe rake-pipeline does it). So I would rather leave the basic options around and sprockets can extend it further with its own.

Member

josh commented Oct 18, 2012

@josevalim the annoying issue is that assets is attr_accessor instead of a property on the ordered options object. Inorder to define my own options, I need to undef that accessor.

I think rake-pipeline should just bring its own options that it cares about.

Member

josh commented Oct 18, 2012

Alright, don't really care now, I'm going to have to wipe the current config.assets anyway for old rails. So whatever config.assets are initialized by rails will just be ignored by the plugin.

@josh josh closed this Oct 18, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment