-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add :injector_mixin
plugin
#221
base: main
Are you sure you want to change the base?
Conversation
My big question here: what do you think about the name? I'm definitely not wedded to Now that I think about it, AFAIK "mixin" is not really an "official" term within Ruby, so perhaps |
def initialize(name: "Deps") | ||
@name = name | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubocop is saying:
Lint/MissingSuper: Call `super` to initialize state of the parent class.
Do we really think we need to do this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling we should always delegate to super
. Who knows where this class can end up being used later? If it doesn't properly delegate, this would break expectations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My question is more about whether it's really necessary when inheriting from Module
. We already don't do it across the other instances of Module
inheritance in this codebase, and it's not something that's appeared necessary to me when using this technique elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timriley I doubt it plays a role in this particular case. It's not a problem to have super
calls to quell rubocop, though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's beneficial to have a habit of calling super, even if it's a noop. It may happen that you really need to call super, but you don't have this habit and you end up debugging why things don't work just realize you forgot to call super. Been there done that 🙂
d973260
to
3115a9d
Compare
can I use different strategies with this? I usually have something like Import = injector(dynamic: env.eql?('test')) # default strategy is effects provided by Dry::Effects::System::Container
KwargsImport = injector(effects: false) # here opt out back to kwargs |
@flash-gordon hmm, no! I've not seen this kind of advanced use case before. Would you have ideas about if/how we could support it? |
API-wise I'd expect |
The
:injector_mixin
plugin makes it an easy one-liner to define a constant for the given container's auto-injector mixin.This way you never have to worry about doing
Deps = MyContainer.injector
anywhere.It looks like this:
By this point, there will now be a
MyApp::Deps
defined, which you can mix into your component classes as usual, e.g.include Deps["some_dependency"]
.You can also customise the name:
use :injector_mixin, name: "Inject"
will createMyApp::Inject
use :injector_mixin, name: "Nested::Inject"
will createMyApp::Nested::Inject
use :injector_mixin, name: "::Inject
will create::Inject
The mixin constant is defined in the container's after-configure hook, since we rely on the inflector for camelizing and constantizing, and we want to give the user an opportunity to provide a custom inflector.