Support all auto inject strategies #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it possible to provide the underlying injector with a custom strategies container (and any other config that may be made available in the future). This requires some underlying changes, however: the `Inject` constant on `Container` subclasses is no longer set up automatically, since we can’t infer whether the user will want to pass their own options to the injector. Instead, the recommend practice should be that the user creates their own constant to store their preferred injector.
| def kwargs | ||
| strategies[:kwargs] | ||
| def respond_to_missing?(name, _include_private = false) | ||
| injector.respond_to?(name) |
There was a problem hiding this comment.
I've realised that this should actually just be #respond_to? inside a BasicObject subclass (since it doesn't actually offer #respond_to? by default). Will push up this adjustment shortly.
This is because we’re inheriting from BasicObject, which doesn’t provide a respond_to? implementation.
|
I'll merge this in now since all the code tweaking is done and there was no issue raised with the API adjustment :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow
Dry::AutoInjectoptions to be passed toDry::Component::Container#injector. This allows the underlying auto_inject object to be configured as the user requires, while still retaining dry-component's desired behaviour of lazily loading dependencies.The immediate benefit of this is that a user can now set up an injector with their own custom strategies container. However, it does mean that any options added to dry-auto_inject in the future will automatically be available for dry-component's injector as well.
This requires a breaking behavioural change, though. The
Injectconstant that we were setting up automatically onDry::Component::Containersubclasses can't be set up in the same way anymore, since there's no way we can infer by that time if the user wants any particular options to be passed. The recommended behaviour, instead, would be to have the user explicitly define their own constant to store their own preferred injector (e.g.MyApp::Inject = MyApp::Container.injector(my_options)). I think this is a reasonable requirement of users, though, and it was actually what we required of them until the last 0.3.0 release of dry-component, anyway.Sound OK?