Skip to content
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

Should all injectable types require an interface? #12252

Closed
Tracked by #11899
aaronc opened this issue Jun 14, 2022 · 2 comments
Closed
Tracked by #11899

Should all injectable types require an interface? #12252

aaronc opened this issue Jun 14, 2022 · 2 comments
Labels
C:depinject Issues and PR related to depinject S:proposed T: Dev UX UX for SDK developers (i.e. how to call our code)

Comments

@aaronc
Copy link
Member

aaronc commented Jun 14, 2022

Currently in depinject, one-per-module and many-per-container types must implement the OnePerModuleType and ManyPerContainerType interfaces. These interfaces actually serve as good indicators to users for what they can put into and out of the container.

It might be useful for debugging purposes to require such an interface for all types that can be injected. Currently, we need to communicate with documentation how types are supposed to use. If we required an Injectable interface for other injectable types (besides one-per-module and many-per-container), depinject could return an error before it even tries to resolve dependencies that a type isn't intended to be injected.

The downside of this is that types that currently don't know about depinject couldn't be injected directly and instead would need a wrapper type.

Hard to say if the benefits outweigh the downsides. Would be good to get feedback on usability as we work with this tool.

@julienrbrt
Copy link
Member

I agree that it would be helpful for debugging, when you can know directly from the editor what is injectable.
When learning how to use depinject, I've struggled to understand what could be injectable. It might be even harder when you aren't directly in the sdk codebase to see what modules provide as injectable outputs.

However, I wonder if the added burden and extra code worth it. This might as well be solved by good documentation. 🤷🏾‍♂️

@julienrbrt julienrbrt added the T: Dev UX UX for SDK developers (i.e. how to call our code) label Jun 15, 2022
@aaronc
Copy link
Member Author

aaronc commented Jan 23, 2023

I think the time for adding this has probably passed so closing this. We should probably be tagging depinject as 1.0 at this point.

@aaronc aaronc closed this as completed Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:depinject Issues and PR related to depinject S:proposed T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

No branches or pull requests

4 participants
@aaronc @tac0turtle @julienrbrt and others