-
Notifications
You must be signed in to change notification settings - Fork 207
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
Cyclic dependencies in decorators #391
Comments
Thanks for reporting this, this looks like a bug. I'll investigate. Internal ref: GO-2096. |
I'm curious about the solution for this. I propose this one:
Please let me know your thoughts on this. I could raise a PR with this solution. |
Hi @JacobOaks. Just wanted to check up on where we are with this fix. I'd be happy to help with it. |
Hey @paullen - sorry for the delayed response. This is a legitimate issue we should fix, but we are still thinking about the right way to go about it, since it has potential to break existing apps. |
Describe the bug
Providing a decorator with params other than the decorated type does not use the singleton value already in the container but creates another instance of it. The entire point of DI, imo, is the singleton pattern of type usage. Please let me know if this is a design choice and some context around why this way was chosen, if you can.
To Reproduce
Consider the following code:
Expected behavior
The expected behaviour should be an error while calling the
Decorate
function letting the user know about the cyclic dependency.Additional context
I have an implementation of decorators with support for cyclic checks and multiple decorators per type in a single container in my fork. Although it is a little hacky since it was done when I was just starting out, we could formalise the design and I could raise a PR with these features. Let me know if this is something y'all would like. There is a need for multiple decorators per type with the cyclic dependency check in my organisation and I hope to see these features in the original repo.
The text was updated successfully, but these errors were encountered: