HampusMat/Syrette

Add a way to bind a specific instance of a singleton

Opened this issue · 9 comments

Hi! I'm loving this crate so far, so thank you for all your hard work. There's one thing which it seems to be missing to fit my use case, though:

It would be nice to be able to bind a type to an existing singleton, like so:

let my_singleton = MySingleton::new();
my_singleton.configure(/*args*/);
my_singleton.do_something_else();

di_container.bind::<MySingleton>::().to_singleton(my_singleton);
// And now we can use the specific `my_singleton` instance when resolving dependencies with the container.

Otherwise, we either have to use the factory feature, which forces rust nightly, or depend solely on the dependency's new function, which would make it quite challenging to configure the service based on external information.

There's a tentative PoC here: https://github.com/TechnoPorg/Syrette/tree/explicit-singleton-binding/. But I'm not especially familiar with Syrette, so I'm sure there's much there that I left out. For one thing, the injectable derive macro still requires a new constructor on my branch, even though it technically isn't needed.

Thanks!

Wow i actually somehow haven't thought of this use case. Thank you for bringing it to my attention.

I'm not sure that the binding builders should have control over the scope of bindings. That's more the responsibility of the scope configurators.

I think using a default factory is the best approach here. That way, no Injectable impl is needed. I just realized that the to_default_factory functions doesn't need Rust nightly to work since the function they take doesn't have a generic number of parameters. That's the reason Rust nightly is needed for factories if you were wondering. The number of parameters cannot be generic when the Fn/FnMut/FnOnce() syntax is used but it can when the normal syntax for passing type parameters is used. And to use it, the unboxed_closures Rust feature flag is needed.

Anyway, it should be possible to make the to_default_factory functions only use the Fn/FnMut/FnOnce() syntax. Then it can be removed from the factory crate feature. Support for configuring the scope after calling to_default_factory will also be needed since right now they return "when configurators".

I won't be able to do this in a while because I'm currently on vacation nowhere near my computer but i'm glad to help you with it. Just ask if you have any questions

Thanks for the feedback!

You would know best, but it seems like to_default_factory is not the right fit for a oneshot factory that produces a singleton. Since to_default_factory takes a &Fn, the user can't move ownership of anything into the factory function, which would force them to clone things unnecessarily before sending it into a function that only gets used once.

Anyways, I got started on a branch here: https://github.com/TechnoPorg/Syrette/tree/factory-no-nightly

Oh i didn't think about that. It should be possible to make it not take a reference and instead have CastableFactory store it in a box. That should solve this i think

First, @HampusMat Thanks for making this library! I think it has a lot of potential.
Do you have any timeframe of when this issue is going to be resolved? I would need to inject a constant in my project.

InversifyJS (since README indicate inspiration from InversifyJS), has a feature to bind constant.
https://github.com/inversify/InversifyJS/blob/master/wiki/value_injection.md

It would be very useful to have that implemented.

First, @HampusMat Thanks for making this library! I think it has a lot of potential. Do you have any timeframe of when this issue is going to be resolved? I would need to inject a constant in my project.

InversifyJS (since README indicate inspiration from InversifyJS), has a feature to bind constant. https://github.com/inversify/InversifyJS/blob/master/wiki/value_injection.md

It would be very useful to have that implemented.

It should be possible relatively soon but i think i want to also complete #24 because then #11 should be doable as well as i had some trouble with the nested functions while trying to implement that. These changes would make the factory API stable & unchanging. I'm not sure though as that will make the the release delayed quite a bit

First, @HampusMat Thanks for making this library! I think it has a lot of potential. Do you have any timeframe of when this issue is going to be resolved? I would need to inject a constant in my project.
InversifyJS (since README indicate inspiration from InversifyJS), has a feature to bind constant. https://github.com/inversify/InversifyJS/blob/master/wiki/value_injection.md
It would be very useful to have that implemented.

It should be possible relatively soon but i think i want to also complete #24 because then #11 should be doable as well as i had some trouble with the nested functions while trying to implement that. These changes would make the factory API stable & unchanging. I'm not sure though as that will make the the release delayed quite a bit

No nevermind about #11 that can wait so at least #24 and #25.

Max two weeks though i can't promise anything

Should i add a to_value binding builder function that takes a clonable value? It would just call to_dynamic_value with Clone::clone so it would be kind of redundant but also a bit more convient. I don't know

I think that a to_value function might hide the fact that the value is getting cloned from consumers of the crate. It might be better to make them explicitly clone the value, so that it is always clear how the value is being used.

I think that a to_value function might hide the fact that the value is getting cloned from consumers of the crate. It might be better to make them explicitly clone the value, so that it is always clear how the value is being used.

Right. That's reasonable. I can add it if anyone asks for it in the future but with a name that signifies that it clones the value