DiscourseSassImporter seems to break ActiveAdmin

(Thomas Smyth) #1

Hello. I’m using ActiveAdmin as an admin panel for my plugin. In newer versions of Discourse I’m getting:

File to import not found or unreadable: active_admin/mixins.
Load paths:

The same path is repeated 37 times.

The offending code is in an app/assets/active_admin.css.scss file that ActiveAdmin added:

@import "active_admin/mixins";
@import "active_admin/base";

From what I can tell, the files are not being found because DiscourseSassImporter has its root parameter hard coded to the above value, so Sass::Tree::ImportNode is searching the same path 37 times. I’m not sure why this was done, but it seems that it’s preventing Sass from searching all the correct load paths and resulting in the above error.

@neil, it appears you originally wrote the DiscourseSassImporter, and @eviltrout you’ve made commits to it. Can either of you offer any suggestions here? I would be much obliged.

(Robin Ward) #2

Hmm you are on to something here. It seems SassImporter does break non-discourse Sass assets. We should probably fix this to be good Internet citizens. @neil did you write this or was it @sam?

(Neil Lalonde) #3

I wrote that code. I’ll have a look.

(Neil Lalonde) #4

@tomsmyth1 I tried installing ActiveAdmin according to their docs and it failed both ways.

gem 'activeadmin', github: 'gregbell/active_admin' and gem 'activeadmin', github: 'gregbell/active_admin', branch: '0-6-stable'

What’s the trick?

(Thomas Smyth) #5

What is your error? My Gemfile line is this: gem ‘activeadmin’, :git => ‘GitHub - activeadmin/activeadmin: The administration framework for Ruby on Rails applications.

I think this should happen with any Gem that tries to @import some stylesheets, but maybe those are rare…

(Neil Lalonde) #6

I get this:

$ bundle exec rails g active_admin:install --skip-users
2014-08-06T19:29:53Z 24484 TID-owmek6zt8 INFO: Sidekiq client with redis options {:url=>"redis://localhost:6379/0", :namespace=>"sidekiq"}
/Users/neil/.rvm/gems/ruby-2.0.0-p247-turbo/gems/activesupport-4.1.4/lib/active_support/dependencies.rb:478:in `load_missing_constant': Circular dependency detected while autoloading constant Base (RuntimeError)
	from /Users/neil/.rvm/gems/ruby-2.0.0-p247-turbo/gems/activesupport-4.1.4/lib/active_support/dependencies.rb:180:in `const_missing'
	from /Users/neil/.rvm/gems/ruby-2.0.0-p247-turbo/gems/activesupport-4.1.4/lib/active_support/dependencies.rb:512:in `load_missing_constant'
	from /Users/neil/.rvm/gems/ruby-2.0.0-p247-turbo/gems/activesupport-4.1.4/lib/active_support/dependencies.rb:180:in `const_missing'
... snip ...

(Thomas Smyth) #7

Ah yes, now I remember! Add this to your Gemfile:

# this gem is a dependency of activeadmin and I forked it because there is a chunk where
# it tries to guess a class name based on a controller name, and active admin has a controller named
# 'BaseController', so it guesses 'Base', which causes all hell to break loose because there are so many Base's in this project
# so I added some checks in inherited_resources to stop it from trying to find 'Base'
# posted an issue on activeadmin github but no responses yet.
# not sure what the long term solution is but this works for now.
gem 'inherited_resources', :git => 'https://github.com/hooverlunch/inherited_resources'

(Neil Lalonde) #8

So it doesn’t namespace its stuff and uses very generic names… yikes.

(Thomas Smyth) #9

Perhaps. Honestly I can’t quite remember what it was trying to do. The good news is that’s not what we’re up against. The style bug is unrelated.

(2rba) #10

I have the same issue with the spree
I have manage to fix this by changing DiscourseSassImporter initialize to:

  def initialize(*args)
    if args.count == 2
      @context = args.first
      super args[1].to_s
      @context = args.first unless args.first.is_a? String
      @root = Rails.root.join('app', 'assets', 'stylesheets').to_s
    @same_name_warnings = Set.new

Is works now, but is this change OK ?

(Thomas Smyth) #11

I confirm @2rba’s solution works for me. Very clever! ActiveAdmin now works fine and Discourse seems no worse for the wear. Can this be incorporated into master?

I would submit a pull request, but I don’t quite get how @2rba’s code works. @2rba, would you care to comment?

(2rba) #12

Hello Tomas,

Discourse overwrite initialize method, and that cause problems to other apps.
The code check if there are 2 arguments then there is a non-discourse call and super is called
else there just copy of discourse initialize method

(Thomas Smyth) #13

I see. So for some reason, Discourse calls have more than 2 (or is it less than 2?) arguments? What is the reason? I’d just like to be able to write a good comment.

(2rba) #14

Discourse calls with one argument.
No idea why it was changed, It was a quick fix.

(Thomas Smyth) #15

Bumping this one. It’s been awhile, but it seems like this is still an issue.

Has anyone thought more about how to prevent DiscourseSassImporter from breaking non-discourse assets? I still have @2rba’s monkey patch in my code.