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

Factory function returning other di.Provide #51

Open
matdurand opened this issue Dec 8, 2023 · 7 comments
Open

Factory function returning other di.Provide #51

matdurand opened this issue Dec 8, 2023 · 7 comments

Comments

@matdurand
Copy link
Contributor

matdurand commented Dec 8, 2023

Sorry for the multiple questions. I did search through the previous issues, but could not find what I was looking for.

I'm trying to have a function that would return a variable number of dependencies to add to the DI container, but where the function also received some dependencies from previously declared di.Provide. I saw this answer which seems to hint that it's possible for a di.Provide to return di.Option, but I cannot make it work.

Here is an example:


type moduleConfig struct {
	di.Inject
	cfg1 string `di:"cfg=m1.v1"`
	cfg2 string `di:"cfg=m2.v1"`
}

func provideModules(cfg moduleConfig) di.Option {
	//imagine some hypothetical conditions to decide which modules to create. 0-n modules can be created
	//using moduleConfig to decide...
	return di.Options(
		di.Provide(m1.NewModule1, di.As(new(component.Module))),
		di.Provide(m2.NewModule2, di.As(new(component.Module))),
	)
}

func startModules(modules []component.Module) error {
    //... do something with the declared modules
    return nil
}

func main() {
	c, err := di.New(
		di.ProvideValue("aaa", di.Tags{"cfg": "m1.v1"}),
		di.ProvideValue("bbb", di.Tags{"cfg": "m2.v1"}),
		di.Provide(provideModules),
	)
	if err != nil {
		panic(err)
	}

	err = c.Invoke(startModules)
	if err != nil {
		panic(err)
	}
}
@matdurand
Copy link
Contributor Author

matdurand commented Dec 10, 2023

I got it working by using Invoke and injecting to container to register new dependencies. Is there an easier way?

func provideModules(c *di.Container, cfg moduleConfig) error {
	//imagine some hypothetical conditions to decide which modules to create. 0-n modules can be created
	//using moduleConfig to decide...

	err := c.Provide(m1.NewModule1, di.As(new(component.Module)))
	if err != nil {
		return err
	}

	err = c.Provide(m2.NewModule2, di.As(new(component.Module)))
	if err != nil {
		return err
	}

	return nil
}

//...

c, err := di.New(
    //...
    di.Invoke(provideModules),
)

@matdurand
Copy link
Contributor Author

matdurand commented Dec 11, 2023

Another issue I found with using di.Invoke inside di.New is if you need a parent container. The problem is that I need to create the container to be able to call AddParent, so if any .Invoke calls passed to di.New need some dependencies from the parent, it will fail. To solve this, I need to revert to creating an empty container, adding the parent, and then calling all the .Provide and .Invoke in sequence. With error management for each call, it gets quite verbose.

One useful addition would be to have an option that allows to pass the parent.

rootDI, err := di.New(...)
	
c, err := di.New(
	di.Parent(rootDI),
	di.Invoke(provideModules),
)

Happy to create a PR if you think that could work.

On the same topic about parents, there is also an issue if you have a container with a parent and you try to invoke or provide something that needs to get the di.Container injected, since the parents are of the same type, we get a group and cannot inject it.

Example:

func provideModules(c *di.Container) error {
	return nil
}

func main() {
	rootDI, err := di.New()
	childDI, err := di.New()
	err = childDI.AddParent(rootDI)
	if err != nil {
		panic(err)
	}
	err = childDI.Invoke(provideModules)
	if err != nil {
		panic(err)
	}
}

fails with error multiple definitions of *di.Container, maybe you need to use group type: []*di.Container.
I understand why it fails, but I'm hoping for a nice workaround for this issue, since I'm using .Invoke to inject the di container in the example in my previous comment, and when I add a parent, it doesn't work anymore. I'm hoping for something better than injecting a slice and accessing an element directly.

@matdurand
Copy link
Contributor Author

Hey @defval, any thoughts on this?
The root issue here is how to have a function that receives some existing dependencies and return new dependencies to be added to a di.Container.

I can probably cook some utility function, but I would like to know how you would solve this first.

@matdurand
Copy link
Contributor Author

I did some thinking and maybe adding a ProvideOptions function would solve this without much impact on the library API. And it could kill 2 birds with one stone as it solves the conditional injection and it avoids the need to inject the di.Container using Invoke, so we skip the issue with the parent/child both providing a di.Container. The parent/child would still need solving, but that would be a edge case now.

type Backup struct{}

type Cfg struct {
	BackupEnabled bool
}

func provideOptionalDependencies(cfg *Cfg) ([]di.Option, error) {
	if cfg.BackupEnabled {
		return []di.Option{di.ProvideValue(Backup{})}, nil
	}
	return nil, nil
}

func main() {
	c, err := di.New(
		di.ProvideValue(Cfg{BackupEnabled: true}),
                // every di.Option returned by this function are applied to the container
		di.ProvideOptions(provideOptionalDependencies),
	)
	if err != nil {
		panic(err)
	}
	
	//...
}

Thoughts?

@matdurand
Copy link
Contributor Author

Created a PR to propose a solution
#53

@defval
Copy link
Owner

defval commented Jan 1, 2024

Hello @matdurand,

Sorry for the delayed reply. I've reviewed your suggestion of using a ProvideOptions function for conditional injections and find it aligns well with library's design.

Additionally, I've been contemplating a When/If provide option for conditional provides, which involves a pre-provide check to decide whether a dependency should join the graph. For example:

type Backup struct{}
type Cfg struct {
    BackupEnabled bool
}

func IsBackupEnabled(cfg *Cfg) (bool, error) {
    return cfg.BackupEnabled, nil
}

func main() {
    c, err := di.New(
        di.ProvideValue(Cfg{BackupEnabled: true}),
        di.ProvideValue(Backup{}, di.When(IsBackupEnabled))
    )
    if err != nil {
        panic(err)
    }
    //...
}

This method could offer a more streamlined approach to managing conditional dependencies. Currently, I'm unable to dedicate time for development and testing of this feature, but I'm open to contributions.

I'm interested in your opinion: how do you see the di.When provide option in comparison with the di.ProvideOptions method you proposed?

@matdurand
Copy link
Contributor Author

Hey @defval,

At first glance, I like the di.When approach because it's very explicit, but after a bit of thinking, it seems more specialized than the ProvideOptions method. Sure ProvideOptions can be used to create conditional dependencies, but it can also be used to provide a list of any dependencies. However, on the top of my head, I cannot find any real life use cases beside conditional dependencies for which returning a list of Options would be useful.

From an implementation standpoint, it seems obviously a bit more complex to implement di.When since the library would need to interpret the di.When. Would you evaluate the When immediately and just discard the provided constructor if the result is false? or is there any value to keeping track of "false" dependencies, for logging/debugging purposes? But anyway, all of this is more complex than the ProvideOptions alternative.

There are just my initial thoughts.

Repository owner deleted a comment from tim-tepia Mar 18, 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

2 participants