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

Question: Doubled references when injecting #152

Closed
intellix opened this issue Dec 20, 2023 · 3 comments
Closed

Question: Doubled references when injecting #152

intellix opened this issue Dec 20, 2023 · 3 comments
Assignees
Labels
question Further information is requested

Comments

@intellix
Copy link

intellix commented Dec 20, 2023

Is your question related to a contrib/ feature?

No

Question

We're currently using typedi and were looking at esbuild but were stuck due to reflect-metadata not being supported. A friend recommended this library as an alternative and I couldn't help but notice in the example that dependencies need to be referenced twice:

typedi (original - also similar to how it looks in Angular):

@Service()
class RootService {
  constructor(private logger: LogService) {}
}

this library:

@Service([LogService])
class RootService {
  constructor(private logger: LogService) {}
}

The fact that you have to declare all services twice (once in the array of the @Service([]) decorator and again in the constructor) is a bit off putting to me cause it's going to add a lot of boilerplate and it'll require quite a bit of effort to migrate. I know that once I migrate, people are going to forget to maintain both the array + constructor and end up with weird errors and constantly be asking why stuff doesn't work.

Is this a technical limitation of Stage 3 decorators or more of a design decision? One thing that Angular 16 added recently was the ability to inject without the constructor like so:

@Service()
class RootService {
  private logger = inject(LogService);
}

This is nice cause you don't need to match constructor args when extending classes etc.

@intellix intellix added the question Further information is requested label Dec 20, 2023
@freshgum-bubbles
Copy link
Owner

freshgum-bubbles commented Dec 20, 2023

@Service(...) boilerplate

The fact that you have to declare all services twice (once in the array of the @service([]) decorator and again in the constructor) is a bit off putting to me cause it's going to add a lot of boilerplate and it'll require quite a bit of effort to migrate.

It definitely means more boilerplate, but in my experience it's a lot easier to maintain than the original TypeDI. Issues like interfaces being represented as Object in the reflect metadata were initially quite hard to debug, and there were a lot of other pitfalls to the DX.

I'm not really sure what a better solution is, as of now; the current implementation is a lot easier in a few ways due to its avoidance of "magic" (e.g. reflect metadata), which can fail in unexpected and mysterious ways.

Typing / Runtime errors

Yeah, agreed -- this definitely needs an eager eye. Due to backwards compatibility with the original TypeDI (and due to TypeScript bugs), the Service decorator doesn't ensure the signature of the implementation's constructor aligns with that of the dependencies passed -- TypedService does, so it's a pretty welcome addition here.

Stage 3 Decorators

Is this a technical limitation of Stage 3 decorators or more of a design decision?

A design decision thus far. I haven't investigated much of the Stage 3 decorator surface, so I'm not entirely sure how well they jibe with experimental decorators' type metadata (and how well it works in general). I'm really looking to avoid the pitfalls of prior solutions altogether, as they can be very frustrating to debug (and can't really be flagged at compile time).

The package does support ES Decorators, but not property injection, etc. It just wraps the Service function to provide compatibility with the new decorator API.

inject(...)

@Service()
class RootService {
  private logger = inject(LogService);
}

I'm very heavily against this approach.
That links to a discussion I had regarding inject a while ago.

It's effectively the service locator pattern: you're obscuring the dependencies of your class
by taking in a container and retrieving dependencies from it manually. That's the framework's
job, and in most DI solutions it does a much better job of it.

There are a few (practical, not just theoretical) issues with inject:

  • The dependencies of services are obfuscated; now it takes in a container and does... something (this is called the service locator pattern). This is generally a bad idea, especially when working with external modules.
  • It makes instantiating these classes (manually) much more confusing: do you need to setup global state? The state required now isn't explicitly in the class's signature, so you need to read the source code to understand how it works (eww).
  • The framework has absolutely no idea what the class depends upon prior to instantiating it, so you lose out on any capacity to perform checks before accepting the class into a container.
  • The beauty of this package is each class is dependencies in, service out: obscuring that contract is an anti-pattern, and it's something I'd rather not do just to save a few keystrokes.
  • Some global state trickery would be required to use inject with non-default containers. It might even mean introducing a custom class instantiation function into the package, to set which container inject uses -- this is pretty gross.

So, inject is pretty much a no-go. Just too many pitfalls.
Interested in alternatives to cut down on boilerplate though.

Nevertheless, if you're hellbent on destruction, you could use HostContainer :-)

freshgum

@freshgum-bubbles
Copy link
Owner

You seem to be set on your way so I'm going to close this :)

Feel free to report any issues / ask more questions though!

Copy link
Contributor

github-actions bot commented Feb 6, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants