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

Add support of Value Groups #52

Open
samber opened this issue Dec 20, 2023 · 5 comments
Open

Add support of Value Groups #52

samber opened this issue Dec 20, 2023 · 5 comments
Milestone

Comments

@samber
Copy link
Owner

samber commented Dec 20, 2023

@matdurand :

Hey, great work on the lib!

Any chance you have plans to support a concept similar to groups in Dig? (aka, registering a bunch of things of the same type, and asking for the list later). Pretty useful to do something like register a bunch of Routes associated with controllers in a DI container and then asking for all the routes to register them. That is super useful to decouple route creation, controller creation and servers for example.

https://uber-go.github.io/fx/value-groups/

@samber samber mentioned this issue Dec 20, 2023
50 tasks
@matdurand
Copy link

What is the value compared to do.ProvideValue(injector, []gin.IRoute{route1, route2, route3})? Do you sometimes use a value in a standalone fashion or always in a group?

do.ProvideValue is an eager loading, would you prefer injecting a provider, such as do.ProvideGroupItem(injector, NewRoute1)?

Injecting a type many times in different places, and then invoking them altogether looks interesting.

That last sentence captures the main difference between ProvideValue and something like ProvideGroupItem. The major difference is that populating the di container should not be centralized. I could create multiple IRoute in multiple contructors. If I need to append them all in a slice in a central place before injection, I need something to be aware of all the route constructors. Adding something like do.ProvideGroupItem would help yeah. We would have to think about what is the best API to achieve that, but you get the idea.

I'm gonna try to think about a possible API to achieve this.

@samber
Copy link
Owner Author

samber commented Dec 21, 2023

Here are some ideas about api 👇

Dedicated helpers:

  • ProvideGroupItems[*Wheel](injector, ...provider)
  • ProvideTransientGroupItems[*Wheel](injector, ...provider)
  • ProvideGroupItemsValue[*Wheel](injector, ...services)
  • InvokeGroupItems[*Wheel]() []*Wheel
  • InvokeGroupItemsAs[ICarPart]() []ICarPart
  • MustInvokeGroupItems[*Wheel]() []*Wheel

Reuse existing helpers:

  • Provide[*Wheel]()
  • Invoke[[]*Wheel]()

Note:

  • In the second proposition, we would need to keep track of all provided *Wheel, which is not possible today (and not suitable?)

  • I don't think "named" providers are appropriate here since it will introduce lots of confusion and we must keep control on type safety.

  • "Override" providers are cool for testing, but in the current implementation, do does not support service deletion (only upsert).

  • I think we should support implicit aliasing as well: https://do.samber.dev/docs/service-invocation/accept-interfaces-return-structs

  • Explain+Describe helpers must be adapted.

Injectors:

About implem, we have 2 options:

  • Creating a new service type containing services and providers. In that case, it would be local to a scope.
  • Creating a new service type similar to explicit aliasing (kind of proxy having). If services are localized in parent scopes: ok.
  • Creating no service type and just look for services at invocation in current and ancestors scopes (like implicit aliasing).

I would be in favor of one of the last 2 options. FYI, to date, we are not able to inject a service twice in a scope.

@samber
Copy link
Owner Author

samber commented Dec 21, 2023

Other implemention possibility: having a service of type "group" containing the providers, that invoke parent scopes recursively and load all items whatever the scope of injection.

@matdurand
Copy link

Some thoughts:

  • Ideally we use the same API that already exist, as introducing new helpers increases complexity and the barrier of entry for the library. But looking at the current API, I think it will be hard to do this without new helpers for in Invoke part. But for the Provide part, would there be a way to introduce some kind of Option parameter to the current API to mark something as part of a Group. Then if you register multiple times the same type, but as part of a group, it would just append to the group instead of panicking?

  • I'm gonna link back to something from your documentation, for the "Service aliasing". The doc says: "The first matching service in the scope tree is returned.". Seems like there might be an opportunity here to leverage this and return a slice instead of "the first match". I don't have a lot of experience with your lib, but it seems that the current behavior might be a source of bugs. Maybe this relates to Group as well, we there is the possibility of multiple matches to an interface, and I could want to get all the possible matches, same as getting all the possible values for a group. I understand that it's convenient for MustInvoke to return a single value, otherwise it would be cumbersome to work with for the standard singleton use cases, so maybe there is no choice but to introduce a new helper for Group.

I'm gonna continue my reflection during the holidays...

@svenwltr
Copy link

I am currently looking into this project and started directly with using v2.

IMO the Value Groups would be very useful and wanted to share my two use cases:

  1. There are a bunch of "workers", which implement interface{ Run(context.Context} error }. I want to execute them all at once in an error group and currently I am required to specify the workers in the provide function and then I need to invoke them again to pass them to the error group.
  2. There are a bunch of controllers, which implement interface { Register(router chi.Router) }. They can hook into the router and register paths by themselves.
# example controller
func (c *FooController) Register(router chi.Router) {
	router.Get("/bar", c.foobarHandler)
}

It would be nice if I would be able to get every provided value which implements a certain interface. Ideally even lazily loaded ones.

This would allow a very slim configuration of injected services, which is basically a list of all needed Provider[T any] functions.

This feature btw looks like a generalization of the Shutdown and Healthcheck features. It would be possible to implement them easily by getting all provided values that implement the Shutdowner interface resp. Healthchecker interface.

@samber samber added this to the v2 milestone May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants