Skip to content

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

Merged
merged 1 commit into from
Jul 23, 2025

Conversation

seuros
Copy link
Member

@seuros seuros commented Jul 23, 2025

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.

@seuros seuros requested a review from Copilot July 23, 2025 18:45
Copy link

@Copilot Copilot AI left a 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.configure method 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

def self.configure
yield configuration
ActiveSupport::Deprecation.new.warn(
Copy link
Preview

Copilot AI Jul 23, 2025

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.

Suggested change
ActiveSupport::Deprecation.new.warn(
ActiveSupport::Deprecation.warn(

Copilot uses AI. Check for mistakes.

Comment on lines +20 to 22
ActiveSupport.on_load(:active_record) do
extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot
end
Copy link
Preview

Copilot AI Jul 23, 2025

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.

Suggested change
ActiveSupport.on_load(:active_record) do
extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot
end
ActiveRecord::Base.extend ClosureTree::HasClosureTree, ClosureTree::HasClosureTreeRoot

Copilot uses AI. Check for mistakes.

@seuros seuros merged commit d18e80c into master Jul 23, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant