-
Notifications
You must be signed in to change notification settings - Fork 247
feat: migrate from ActiveSupport::Autoload to Zeitwerk #457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the gem from ActiveSupport::Autoload to Zeitwerk for module loading to resolve autoloading issues. The change aims to fix NameError: uninitialized constant ClosureTree::HasClosureTree::ClosureTree errors when using this gem with other gems.
- Replaces ActiveSupport::Autoload with Zeitwerk loader configuration
- Removes the configuration system (Configuration class and related generator)
- Deprecates the
ClosureTree.configuremethod with a warning message
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/closure_tree.rb | Core migration from ActiveSupport::Autoload to Zeitwerk, removes adapter loading hooks |
| closure_tree.gemspec | Adds zeitwerk dependency |
| lib/closure_tree/configuration.rb | Removes Configuration class entirely |
| lib/closure_tree/has_closure_tree.rb | Removes database_less configuration check in error handling |
| lib/closure_tree/support.rb | Removes manual require statements for autoloaded modules |
| lib/closure_tree/adapter_support.rb | Removes AdapterSupport module |
| lib/generators/closure_tree/config_generator.rb | Removes config generator |
| lib/generators/closure_tree/templates/config.rb | Removes config template |
| module ClosureTree | ||
| def self.configure | ||
| yield configuration | ||
| ActiveSupport::Deprecation.new.warn( |
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new ActiveSupport::Deprecation instance for each call is inefficient. Consider using ActiveSupport::Deprecation.warn directly or creating a shared deprecation instance.
| ActiveSupport::Deprecation.new.warn( | |
| ActiveSupport::Deprecation.warn( |
| ActiveSupport.on_load(:active_record) do | ||
| extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot | ||
| end |
Copilot
AI
Jul 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context should be ActiveRecord::Base not the block scope. This line should be ActiveRecord::Base.extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot to properly extend ActiveRecord::Base with the modules.
| ActiveSupport.on_load(:active_record) do | |
| extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot | |
| end | |
| ActiveRecord::Base.extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot |
the autoloader is acting in a non predictable manner when i try to implement this gem, with my new gem.
with error : NameError: uninitialized constant ClosureTree::HasClosureTree::ClosureTree
Switching to Zeitwerk fixes this problem.
Removed non-op configuration and deprecated the configuration mechanism.