-
Notifications
You must be signed in to change notification settings - Fork 208
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
Intercept provided values #408
Comments
Hi @asmeikal, thanks for the issue & PR. Dig already supports a callback system where you can register What are your thoughts on extending this system to support the needs you described? It seems like, specifically, you'd like:
I think both of these things can be accomplished in a backwards-compatible way without creating a second callback system. What do you think? |
Hi @JacobOaks, as mentioned in the initial message I wouldn't rely on the existing options, since they are registered at the provider level and would result in a strange interface. If you are suggesting instead to rely on the I'm having these doubts, specifically:
|
Hey @asmeikal! I'd like to hear what other maintainers think, but I'm not necessarily opposed to creating a top-level As for |
Before we add a whole new top-level API for registering a global callback for all providers, could you consider adding the constructed values into CallbackInfo? I think it would be a more contained change than what you're proposing and could still satisfy your requirements. For dealing with providers returning multiple results, I would suggest using a slice of |
Hi @tchung1118, thank you and @JacobOaks both for the feedback. I updated the PR to rely on the existing |
Hey @asmeikal thanks for the update! This looks like a step in the right direction. I do think we should stray away from having a separately stored callback at the scope level though unless there is really strong justification for it as I think it will introduce extra complexity and confusion around callbacks in general. You mention that your use-case is to build a DI framework on top of dig. Is there any reason this framework can't insert the callback w/ every call to Let me know your thoughts! |
@JacobOaks yeah you're right! I updated the PR by removing the container callback. |
Describe the solution you'd like
We need to intercept the provided values after they are constructed, to check for implemented interfaces and possibly call some initialization methods.
We have an implementation ready that adds a
WithProvidedCallback
option to the container that serves this purpose. The callback will receive all values after they are constructed, along with name/group information and type information.Describe alternatives you've considered
This could be implemented as a
ProvideOption
, but would need a list of values as input of the callback, since constructors can return multiple values, and would be duplicated each timeProvide
is called on the container.The callback could be implemented also as a
ScopeOption
to override behaviour in nested scopes, but since it would be the first actual option of this kind, we skipped implementation for now.Is this a breaking change?
No, even if the callback is implemented as a
ScopeOption
.Additional context
We are open sourcing an extremly opinionated dependency injection framework we use to develop microservices in Go. This (small) framework handles automatically health check registration, calling initialization and destruction methods at service startup and destruction, registration of prometheus metrics. Instead of relying on lifecycle hooks like Fx does, our framework relies on interfaces implemented by the provided values, e.g. by looking for the
Start
method on provided values and invoking it at application startup. This reduces code duplication, as most of this shared logic can be implemented with single methods on each component.The text was updated successfully, but these errors were encountered: