ClosureTree/closure_tree

Warning of hierarchy class constant already defined

ryanb opened this issue · 9 comments

ryanb commented

I'm seeing warnings in the log when reloading a page in development after altering a model that has closure_tree added.

/Users/rbates/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/closure_tree-7.1.0/lib/closure_tree/support.rb:36: warning: already initialized constant CategoryHierarchy
/Users/rbates/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/closure_tree-7.1.0/lib/closure_tree/support.rb:36: warning: previous definition of CategoryHierarchy was here

I've tested this in a new Rails app and confirmed it happens in the latest version of Rails (6.0.2.2).

I haven't noticed any other issues, but it would be nice to remove these warnings.

jimhj commented

facing the same warning:
rails 6.0.0
closure_tree 7.1.0

phlegx commented

I have the same warnings.

ruby: 2.7.1
rails: 6.0.3.2
closure_tree: 7.1.0

vutran0111 commented

Have the same problem!

/Users/admin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/closure_tree-7.2.0/lib/closure_tree/support.rb:36: warning: already initialized constant UserHierarchy
/Users/admin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/closure_tree-7.2.0/lib/closure_tree/support.rb:36: warning: previous definition of UserHierarchy was here

ruby: 2.7.2
rails: 6.0.3.4
closure_tree: 7.2.0

kevynlebouille commented

Have the same problem!

/Users/kevyn/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/closure_tree-7.2.0/lib/closure_tree/support.rb:36: warning: already initialized constant SectionHierarchy
/Users/kevyn/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/closure_tree-7.2.0/lib/closure_tree/support.rb:36: warning: previous definition of SectionHierarchy was here

ruby: 2.6.6
rails: 6.1.0
closure_tree: 7.2.0

simi commented

It is probably related to way how hierarchy class is created on the fly

closure_tree/lib/closure_tree/support.rb

Line 36 in 22bbbf1

hierarchy_class = parent_class.const_set(short_hierarchy_class_name, Class.new(ActiveRecord::Base))

and Zeitwerk is not unloading it in rails during reload.

@fxn is there anything we can do about to fix this, any idea?

fxn commented

Hey! I have written a Hello World app following the README. Let's suppose the model is Tag.

TagHierarchy is a top-level constant defined by hand in Object. It is no different than Nokogiri, it is a constant in Object that it has not been autoloaded, therefore, it is not reloaded. Since Object is not reloaded (because it has not been autoloaded), the constants it stores that have not been autoloaded remain there untouched, like String.

I believe there are some options depending on use cases:

  1. If it is expected that users edit the migration of the hierarchy auxiliary table, you may remove_const the constant if present and define it again to have things refreshed.
  2. Alternatively, you can define the constant below model_class, instead of below Object. In that case, when the model class is reloaded, you'll get a fresh new Tag::TagHierarchy again. That is the same principle that applies to autoloaded namespaces, if you have Admin::UsersController, just by unloading Admin, the child UsersController is gone, and you start everything afresh.
  3. If TagHierarchy is internal and remains untouched for all practical purposes, you could just return hierarchy_class if const_defined? (off the top of my head).

Does that help? Please, do not hesitate to ask more questions!

  • 👍4
gambala commented

Any updates here? Still get these warnings in development environment, because of how Zeitwerk work

ruby: 2.6.1
rails: 6.1.4.4
closure_tree: 7.4.0

simi commented

@gambala No, this problem wasn't tackled yet (sadly). Xavier (author of Zeitwerk) pointed out 3 possible solutions. Personally I would go with 1. for now. But I'm not 100% sure this is 100% backwards compatible change.

As a proper solution I think this gem should not define hierarchy model on the fly, but let user/generator create model (backed by classic class in file) and just include ClosureTree::Model concern in there.

ℹ️ btw. definition happens here

closure_tree/lib/closure_tree/support.rb

Line 36 in f3b33f8

hierarchy_class = parent_class.const_set(short_hierarchy_class_name, Class.new(model_class.superclass))

fxn commented

While this is still open, you can at least avoid the warning if you want. Just depend on Zeitwerk >= 2.5.0, and throw this in an initializer:

unless Rails.application.config.cache_classes
  # See https://github.com/ClosureTree/closure_tree/issues/362.
  Rails.autoloaders.main.on_unload('Tag') do
    Object.send(:remove_const, 'TagHierarchy')
  end
end
  • ❤️6