freshgum-bubbles / typedi

Elegant Dependency Injection in TypeScript and JavaScript.
http://typedi.js.org/
MIT License
53 stars 3 forks source link

Stop defaulting the container ID parameter to "default" in .of[...] methods #192

Closed freshgum-bubbles closed 4 months ago

freshgum-bubbles commented 4 months ago

This is a breaking change to the core API.

The container no longer provides a default containerId parameter for the following methods:

In retrospect, this was a bad idea. I'll explain this further with the code below.

import { Container } from '@freshgum/typedi';

const c1 = Container.ofChild(Symbol('c1'));

@Service({ container: c1 }, [ ])
class MyService { }

const c2 = c1.ofChild();

c2.get(MyService); // -> ServiceNotFoundError

Ignoring the hints above, the problem with this code may not be obvious at a first glance. While you'd assume the call to c1.ofChild() returns a child container of c1 (which would then inherit its services), it's actually just returned an exact reference to the default container (Container itself.)

This raises a great deal of ambiguity, as it's no longer immediately clear whether a call to these methods does as you would initially expect.

Therefore, type-safe calls to these methods without a container ID will now raise an error at compile-time. The signature of these methods has been changed to require the container ID parameter.

Migrating away from this behaviour is quite simple: replace all calls to X.ofChild() with a reference to the default container (Container).

Whilst I'm not a fan of making breaking changes to the Container, I believe this one makes sense in the pursuit of a safer, more streamlined API -- if you experience any issues with this, please feel free to open a GitHub issue.

changeset-bot[bot] commented 4 months ago

🦋 Changeset detected

Latest commit: 99724e124286a4602221e22fff97a20ac5dc2d41

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ---------------- | ----- | | @freshgum/typedi | Major |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 3 months ago

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.