NoMethodError: private method 'load' called for nil:NilClass

[marcoroth]

Hey @kaspth, thanks for creating this awesome gem!

I've encountered a weird issue on the latest 0.3.0 version. I have a minimal reproduction repo here: https://github.com/marcoroth/conventional_extensions-repro

I guess the main questions is: Am I even supposed to use load_exentions inside a model class under app/models/?

Versions

• conventional_extensions: 0.3.0
• Ruby: 3.1.2
• Rails: 7.0.4

Relevant Files

• app/models/post.rb
• app/models/post/extensions/mailroom.rb

Reproducion Steps

bundle install

rails db:create db:migrate 

rails c

Loading development environment (Rails 7.0.4)
irb(main):001:0> Post.new.mailroom
/Users/marcoroth/.anyenv/envs/rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/conventional_extensions-0.3.0/lib/conventional_extensions.rb:15:in `load_extensions': private method `load' called for nil:NilClass (NoMethodError)

    @loader.load(*extensions)
           ^^^^^

Possible fix?

This seems to fix it locally, but honestly I haven't fully grokked why @loader wouldn't need to be defined in some cases. This "fix" possibly also has some unwanted side-effects since @loader is not going to be cleaned up in the ensure block because of the loader_defined_before_entrance condition.

  def load_extensions(*extensions, from: Frame.previous.path)
-   @loader = Loader.new(self, from) unless loader_defined_before_entrance = defined?(@loader)
+   loader_defined_before_entrance = defined?(@loader)
+   @loader = Loader.new(self, from) unless loader_defined_before_entrance
+   @loader = Loader.new(self, from) if @loader.nil
    @loader.load(*extensions)
  ensure
    @loader = nil unless loader_defined_before_entrance
  end

I assume the defined?(...) is not doing the right thing, it returns a truthy value even if @loader is nil and thus not instantiating a loader.

irb(main):005:0> @loader = nil
=> nil
irb(main):006:0> defined?(@loader)
=> "instance-variable"

Either way, I'm happy to open a pull request if this fix makes sense.

[marcoroth]

After a second thought this might even be enough:

  def load_extensions(*extensions, from: Frame.previous.path)
-   @loader = Loader.new(self, from) unless loader_defined_before_entrance = defined?(@loader)
+   @loader = Loader.new(self, from) unless loader_defined_before_entrance = !@loader.nil?
    @loader.load(*extensions)
  ensure
    @loader = nil unless loader_defined_before_entrance
  end

[kaspth]

@marcoroth thank you for trying out the gem and for the issue! Technically, you shouldn't need to call load_extensions manually in ActiveRecord::Base inheriting classes like in your repro.

But if you do, it should still work. I'll look over this because I want to make sure I understand it properly, thank you for the repro, it's very helpful.

[kaspth]

@marcoroth I couldn't figure out how to reconcile the implementation with being able to support manually calling load_extensions for Active Records so I removed that feature.

I'll fix the other issue open and prep a new release.

[marcoroth]

Thanks @kaspth! I'm wondering if you actually needed to strip out that feature.

Maybe a warning or raising an error like "Calling load_extensions inside an ActiveRecord model is not necessary" would be enough if the class inherited from ActiveRecord::Base?

But either way, I appreciate you looking into it! 🙌🏼