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

Option to resetProviders dependents #193

Open
esroyo opened this issue May 31, 2023 · 9 comments
Open

Option to resetProviders dependents #193

esroyo opened this issue May 31, 2023 · 9 comments

Comments

@esroyo
Copy link
Contributor

esroyo commented May 31, 2023

@young-steveo I'm wondering if It would make sense for resetProviders(names) to also cascade the reset to services that depend on the reseted services. Or at least have an option that makes that automatically.

var UkrainianBarley = function() { this.origin = 'Ukraine'; };
var EthiopianHops = function() { this.origin = 'Ethiopia'; };
var LocalWater = function() { this.origin = 'Local'; };
var AntarcticaWater = function() { this.origin = 'Antarctica'; };
var Beer = function(barley, hops, water) {
  this.debug = () => console.log('Beer origins:', barley.origin, hops.origin, water.origin);
};

var bottle = new Bottle();
bottle.service('Barley', UkrainianBarley);
bottle.service('Hops', EthiopianHops);
bottle.service('Water', LocalWater);
bottle.service('Beer', Beer, 'Barley', 'Hops', 'Water');


bottle.container.Beer.debug();
// "Beer origins: Ukraine, Ethiopia, Local"

// Reseting "Water" service would reset its dependent services
bottle.resetProviders(['Water'], true);
bottle.service('Water', AntarcticaWater);

bottle.container.Beer.debug();
// "Beer origins: Ukraine, Ethiopia, Antarctica"

Of course this example is very simple, but I find It would be useful on complex dependency graphs.
What do you think?

@young-steveo
Copy link
Owner

This is very interesting. It would probably require a lookup table of "what-depends-on-what."

When Bottle resolves the Beer service for the first time via bottle.container.Beer, it deletes the provider and sets the bottle.container.Beer property to be a concrete instance of Beer. This is so that subsequent bottle.container.Beer accesses are just a simple property lookup.

When you later call bottle.resetProviders with the optional "reset" boolean as the second param, bottle would have to iterate over all services (like Beer) that accepted the dependency and delete them so that the providers would regenerate them.

I kinda love and hate this idea. On the one hand, it would be nice if Water got updated everywhere when reset. On the other hand, services that have side effects in their constructors would end up having those side effects play out twice when their dependencies were reset.

I'll have to think about it.

Perhaps the dependency could be wrapped in a proxy so that it can be "mutable." Then we wouldn't have to recreate the services that depended on them.

@esroyo
Copy link
Contributor Author

esroyo commented Jun 4, 2023

Hi Stephen,

Thanks for your ideas. I totally agree there are concerns when reseting providers. In my opinion that is the user responsibility. It is not different than letting them reset by bottle.resetProviders(['Water', 'Beer']). If those services had construction side-effects, they would have to deal with it. Some libraries won't let you even reset providers. But once the possibility is there, you accept those risks.

The mutable/proxy approach, is easy to implement at the userland level:

var Beer = function(proxy) {
  this.debug = () => console.log('Beer origins:', proxy.Barley.origin, proxy.Hops.origin, proxy.Water.origin);
};

var bottle = new Bottle();
bottle.service('Proxy.Barley', UkrainianBarley);
bottle.service('Proxy.Hops', EthiopianHops);
bottle.service('Proxy.Water', LocalWater);
bottle.service('Beer', Beer, 'Proxy');


bottle.container.Beer.debug();
// "Beer origins: Ukraine, Ethiopia, Local"

// Reseting "Water" service would reset its dependent services
bottle.resetProviders(['Proxy.Water']);
bottle.service('Proxy.Water', AntarcticaWater);

bottle.container.Beer.debug();
// "Beer origins: Ukraine, Ethiopia, Antarctica"

However, I feel It goes against the inversion control principles. You are injecting a (kind of) global which might change at any time. An extreme interpretation of this approach may inject the "container" itself, so everything sees the latest versions of all services. Obviously that would be going too far and would defeat the whole point of DI.

Finally, having some kind of proxied behavior at the library level is even trickier/riskier IMHO.

So in the end the idea here is just to add a convenient option on top of the existing Bottlejs functionality.

Here I have a first draft of a possible implementation:
https://github.com/young-steveo/bottlejs/compare/master...esroyo:bottlejs:feat/reset-providers-propagation?expand=1

@young-steveo
Copy link
Owner

@esroyo Looks good to me, but it has a caveat: it only works for services registered via .service(). If the user creates their services via .factory() or .provider(), the dependency list is not updated.

I think there are two routes we can go that would be levels of acceptable:

  1. We could expose a method allowing users to "register" their dependencies inside factories and providers. Then the resetProviders method will also cascade for those services.
  2. We could do something fancy: Right now, the container that is given to the factory (and to the provider's factory) is just a reference to the main container. We could create a temporary "dependency tracker" object that wraps the container and shares the container's interface. It would behave exactly like the container, except when the user accesses a service inside the factory, it would record it as a dependency (it would be aware of the current service being constructed).

Either of these approaches is fine. I like the second option if it is not too complicated.

Thanks for your interest in Bottle! If you want to create a pull request, go for it!

@esroyo
Copy link
Contributor Author

esroyo commented Aug 27, 2023

Hi @young-steveo, I completely missed your last message. I'm very sorry 🙏 😭

I understand all the things you are commenting. I completely agree with you about the 2nd "fancy" option being the most desirable. I will give it a try :)


On a completely different topic. Would you be open to "modernize" the source code of the repo? Port to ESM modules instead of concating JS sources, a modern bundler/test framework, TypeScript, o even Deno? Just want to know how you feel about that? Thanks

@young-steveo
Copy link
Owner

Regarding modernizing the BottleJS source, I have mixed feelings.

I did not know TypeScript when I created BottleJS (almost NINE!? years ago; wow, time flies). The community added the TypeScript declaration file.

These days, I write more code in Go than JavaScript, but when I write JavaScript, I almost exclusively use TypeScript and reach for modern tools like Vite, Vitest, and maybe Cypress for e2e testing.

Last year, I started a TypeScript branch with the plan to entirely rewrite the BottleJS source from scratch in TS. It's not finished, and may never see a release because—and this is vitally important—I don't use BottleJS anymore! ES Modules exporting TypeScript types make IoC so trivial that I no longer require a DI library in my projects.

However, there are 20k weekly downloads of BottleJS via npm, and over 700 repositories and ~150 packages depend on BottleJS, so I will never stop maintaining it, providing security fixes where needed (rare since it has no dependencies aside from dev tools), and responding to the community; at least as long as people still find value in it.

So, while I would certainly appreciate modern tooling, I wonder if the juice would be worth the squeeze for BottleJS.

@esroyo
Copy link
Contributor Author

esroyo commented Sep 2, 2023

@young-steveo totally understand what you talk about. Once you don't use it, It becomes much much harder to invest on it. To the point that It feels not worth. You will provide minimum maintenance while there is noticeable usage of the lib. Something worth praising. Not everyone does that.

In any case I'm glad to know your position. I was not sure about the reasons to keep the code (more or less) as ES5. I heavily use BottleJS at $WORK, and thus have some interest in that modernization. However, as you said, that represents another level of investment and commitment. I'm not sure if or when I will could be able to take that. So that's all for now. I would open a new thread to seriously talk about that topic if the idea settles on me. Thanks a lot for your time :)

@young-steveo
Copy link
Owner

I'll close this issue for now. If you want to take a stab at a PR to reset the tree of providers no matter if the service was registered via service, factory, provider, etc., go for it. I would be welcome to merge it if it works across all of bottle's surface area.

@esroyo
Copy link
Contributor Author

esroyo commented Aug 1, 2024

Hi @young-steveo, when possible would you please push a new version release on the NPM registry? Let me know if something needs to be done and I can help. Thanks 🙏

@young-steveo
Copy link
Owner

Hey @esroyo , this message notification accidentally got overlooked. I'll let you know once that's done, soon.

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