dry-rb / dry-rails

The official dry-rb railtie
https://dry-rb.org/gems/dry-rails
MIT License
270 stars 27 forks source link

Add support for Rails::Engine #49

Open zlw opened 3 years ago

zlw commented 3 years ago

Hi 👋

Note: This PR builds on top on previous PR where I added support for latest dry-system, but I can't do stacked PRs here 😢 Going through the commits (1st commit - previous PR, 2nd - this PR) might give cleaner diff 🙌


Added experimental support for Rails::Engine

Adding dry-rails to monolith Rails app works like a charm 🥇

However, it's not so great when used with Engines :bowtie:

I think each Engine should define its own Container, otherwise we're loosing whole namespace isolation that Engines provide

MainApp::Container.resolve('users.create')        #> Dry::Container::Error: Nothing registered with the key "users.create"
SuperComponent::Container.resolve('users.create') #> #<SomeComponent::Users::Create:0x00007fa6d963e368>
solnic commented 2 years ago

Please rebase this against the latest master as your other PR got just merged 🙂

zlw commented 2 years ago

@solnic done 👌 it should be ready to go now 🙌

solnic commented 2 years ago

@zlw thanks, this is something we should definitely support. Personally I don't use engines at the moment, which means I won't be able to help with this other than to provide some basic feedback in this PR + release it. If we get this merged and released and it turns out it breaks things, you'd have to own it and fix things though. Otherwise I would have to revert the entire feature because I wouldn't have time to deal with any potential fixes.

zlw commented 2 years ago

@solnic no worries, if something arises from this - I'll do my best to fix it 👌 😄