Open esroyo opened 1 year ago
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.
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
@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:
resetProviders
method will also cascade for those services.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!
Hi @young-steveo, I completely missed your last message. I'm very sorry :pray: :sob:
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
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.
@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 :)
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.
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 :pray:
Hey @esroyo , this message notification accidentally got overlooked. I'll let you know once that's done, soon.
@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.Of course this example is very simple, but I find It would be useful on complex dependency graphs. What do you think?