search

dry-rb / dry-configurable

A simple mixin to make Ruby classes configurable
https://dry-rb.org/gems/dry-configurable/
MIT License
410 stars 56 forks source link

0.16.0 breaking change to how settings with a `constructor` behave #150

Open kspurgin opened 2 years ago

kspurgin commented 2 years ago

Describe the bug

Sorry, I don't have the right terms to describe this clearly.

The value of a setting defined with a constructor was stable in 0.15.0. You could call your setting from anywhere in your application and get the same value. What constructor is for and how it behaves is not explained in the documentation, but my understanding from using it was that it would be called the first time you accessed the setting, to set the initial value or update the default value.

In 0.16.0, the constructor seems to be called every time you access the setting in your application.

To Reproduce

I have provided two scripts in this Gist. They are identical except for the dry-configurable version specified in the inline gemfile block.

It defines a dummy class called Something. Then it defines a module called App that extends Dry::Configurable and defines a setting named thing, with a constructor proc that creates a new Something. Then it calls App.thing twice and puts the object_id of each.

Run ruby dry-config-15.rb.

Then run ruby dry-config-16.rb.

Expected behavior

Without any mention of changed behavior in the CHANGELOG, I expect to get the same instance of the Something object when I call App.thing: 

❯  ruby dry-config-15.rb
1160
1160

Instead I get 2 separate instances of the object when I call App.thing subsequently:

❯  ruby dry-config-16.rb
1060
1080

This side effect may be implied by "Improve memory usage by separating setting definitions from config values", but it's really not clear, and it breaks things even if you are not "accessing objects from _settings or the internals of Dry::Configurable::Config"

My environment

timriley commented 2 years ago

Hi @kspurgin, thanks for this report! This behaviour changed with #143. My presumption there is that the constructors would return identical values for any non-cloneable values. In your example, adding cloneable: true to your setting lines would fix it, but either way, this appears to be surprising behaviour, so thanks for sharing.

I'll actually be fixing this as a by-product of https://github.com/dry-rb/dry-configurable/pull/149, so you'll see the previous behaviour soon enough.

I plan to release this either as 0.16.1 or 0.17.0. In the meantime, please feel free to switch back to 0.15.0 temporarily, knowing that this will be fixed in the next couple of weeks.

kspurgin commented 2 years ago

Hi, @timriley. Thanks for the explanation and the tip to add cloneable: true.

If my report is unexpected behavior to you, I'm glad the report was helpful.

If it indicates that I am using dry-configurable in an unexpected/non-recommended way, I'd appreciate knowing that as well. I started using constructors in that way somewhat tentatively, as I wasn't sure what the assumptions/intentions were for it. But it worked at the time... (famous last words!) If this is "off-label" use of the gem, it's probably better long term to figure out another way to achieve what I need.

I feel like this is a place where some added documentation could be be super-helpful. The cloneable option isn't shown in the existing intro and use page, and figuring out what could be set as a default vs. needed to be set via constructor was a process of experimentation informed by looking at the gem code (hopefully in the right place). If it would be helpful, I can try to work up a documentation PR that adds info on some of these things, with the caveat that it might be pretty hand-wavy as I'm still pretty new to a lot of the concepts and approaches y'all are using in the dry-rb gems (but just starting to learn about and try to use them has helped me immensely, so thanks for your efforts!)