Redefining class regression: undefined method `config' for class

[mitnal]

Describe the bug

We use dry-configurable in our Rails project. In 0.9 we had no problems but since the upgrade to 0.11 we get the following error:

NameError: undefined method `config' for class `App'

After a little bit of digging I found out that it is only a problem the second time we define the class (That the class is redefined twice at all could be a Rails autoloading thing).

To Reproduce

require 'dry-configurable'

class App
  include Dry::Configurable
  setting :test
end
# => App

class App
  include Dry::Configurable
  setting :test
end
# => NameError: undefined method `config' for class `App'
# from .../ruby-2.6.5/gems/dry-configurable-0.11.3/lib/dry/configurable.rb:60:in `singleton class'

Expected behavior

(dry-configurable v0.9 behavior, also notice the different return value (nil instead of App))

require 'dry-configurable'

class App
  include Dry::Configurable
  setting :test
end
# => nil

class App
  include Dry::Configurable
  setting :test
end
# => nil

Your environment

• We are in no hurry to upgrade the gem.
• Ruby version: 2.6.5
• OS: Mac/Linux

[flash-gordon]

I wouldn't say a regression per se, dry-configurable is not developed with this case in mind. I think your code worked "by accident", even in Rails environment classes are not meant to be loaded more than once. Yes, they are re-loaded but every time old constants are removed. That being said, it's surprising the included hook is fired twice /cc @solnic

Class.new.include(Dry::Configurable).include(Dry::Configurable)
NameError: undefined method `config' for class `#<Class:0x00007fa984607810>'

[solnic]

I'm not sure what to do in such situations. Raise an error? Emit a warning and fallback gracefully?

[mitnal]

@solnic I am a little biased here, but I would prefer a solution that would behave similar to the 0.9 behavior. Perhaps a graceful fallback with an optional warning?

@flash-gordon thanks for your feedback, I will take a deeper look at why we reload classes multiple times.

[flash-gordon]

@solnic @mitnal I think it should raise an error, nobody should develop an app intentionally allowing their code to be run twice. Even if it's fine for them it's defo not OK for literally everyone else :) We had the same issue in dry-struct actually, now it raises an exception. This was a "breaking change", sort of, but after a while, people stopped creating issues about repeated class definitions (dry-rb/dry-struct#35).

[solnic]

Yeah I agree with @flash-gordon - if your code loads the same class more than once, then you may have issues. Covering it up on the library side doesn't seem to be a solution. I'm closing this in favor of #89