Abstract and Function support for `InjectableIdentifier`
SnowMarble opened this issue · 8 comments
Is there an existing issue for this?
- I have searched the existing issues
Current behavior
Currently, identifier of unit reference does not support Abstract class and Function. Can you consider adding those to InjectableIdentifier?
Abstract type from @nestjs/common
interface Abstract<T> extends Function {
prototype: T;
}
Minimum reproduction code
I believe reproduction code is not needed for this issue.
Steps to reproduce
Passing abstract class or function causes an error.
Expected behavior
Ignoring type error, it seems that passing them still works. I think only adding the type would be fine.
Automock version
2.1.0
Node.js version
Bun 1.0.31
In which operating systems have you tested?
- macOS
- Windows
- Linux
Other
No response
Hello @SnowMarble,
You can easily solve it by casting the type of the subject dependency, here is an example:
your-class.ts
export abstract class UserAbsSvc { ... }
your-unit.spec.ts
import { Type } from '@automock/types'; // Could be also from `@nestjs/common`
const { unit, unitRef } = TestBed.create(...).compile();
const userSvc = unitRef.get(UserAbsSvc as Type<UserAbsSvc>);
Let me know if it worked!
@omermorad I appreciate your reply.
Type casting definitely works. But I think we should be able to use abstract classes and functions without type casting by default. Because this is not an uncommon use case. What do you think about it?
I see, that's actually an interesting scenario, I think it can work. Would you be able to supply some more context regarding the code?
Does the class you are trying to get()
from the unitRef
extends from an abstract class? Or is it the abstract class itself that being resolved from the unit reference?
Suppose this class:
export abstract class UserAbsSvc { ... }
If the following is the case here - how can abstract class act as a dependency of a another class? Or am I missing something?
export class ConcreteClass {
public constructor(private readonly dep: UserAbsSvc) { ... }
}
I'm working on Nestjs. I use an abstract class as a provider, and bind it with an implementation class.
// some.port.ts
export abstract class SomePort {
public abstract foo(): void
}
// some.adapter.ts
export class SomeAdapter implements SomePort {
foo() {
// do something
}
}
// some.module.ts
@Module({
providers: [
{
provide: SomePort,
useClass: SomeAdapter
}
]
})
export class SomeModule {}
This can make me use abstract class as an injection token. This is similar to specifying a string as a provider. I'm sure you already know it.
So, answering your question, what I want to get from get() is not the abstract class. Providing target is just different.
For more information, you can check out the nestjs docs: https://docs.nestjs.com/fundamentals/custom-providers#class-providers-useclass
Alright, I understand now.
It may be beneficial to take a few factors into account when considering this approach:
Typically, abstract classes are used as blueprints for concrete implementations rather than as identifiers themselves. This is because abstract classes are not intended to be directly instantiated. Instead, they serve as templates for subclasses. Unlike interfaces, which are cancled at runtime, abstract classes not.
When it comes to dependency injection and identifiers, it is more typical to utilize concrete classes or other forms of identifiers such as symbols or strings. These references offer a more precise and explicit indication of the dependencies being injected.
If you're interested in finding an alternative method to uniquely identify your dependencies for injection, considering a concrete class or a custom symbol or string token could be a more suitable option. This approach is more in line with the common patterns and conventions found in NestJS and other dependency injection frameworks. Take a look at TSyringe, for instance, or even libraries from various programming languages. Using Java Springboot as an example - it is not possible to use interfaces as identifiers. Instead, you need to provide the DI engine with the specific class you want to resolve.
Based on this, it seems more logical to use casting with Type
rather than including the Abstract
type in the Identifier
type.
What do you think? 🦾
I agree with your point. However, personally, I prefer avoiding @Inject
decorator and managing additional DI tokens. After doing some research, I found that people have used abstract classes for the same reason to me. If we use abstract classes as implemented, we can use them like an interface.
Using type cast might be enough to use, but I think supporting abstract classes is also worth to consider. And, actually, it's not a big deal, you know - just a new type to be added.
If you still think my suggestion is unnecessary, I respect your decision :)
By the way, I'm not a Java dev, but it seems @Autowire
decorator allows interfaces as DI tokens.
I spent some time looking into this. Let me break it down and share the conclusions :)
Consider the following scenario: you have an abstract class SomePort
with various concrete implementations such as SomeAdapter
and SomeOtherAdapter
. This configuration is completely legal and is frequently used to ensure consistency across multiple implementations.
Consider a class SomeService
that requires both SomeAdapter
and SomeOtherAdapter
:
@Injectable()
export class SomeService {
constructor(
private readonly someAdapter: SomeAdapter,
private readonly someOtherAdapter: SomeOtherAdapter,
) { ... }
}
Alternatively, you may apply the dependency inversion concept and utilize the abstract type of the abstract class SomePort
directly (rather than the concrete):
@Injectable()
export class SomeService {
constructor(
@Inject('SOME_ADAPTER') private readonly someAdapter: SomePort,
@Inject('SOME_OTHER') private readonly someOtherAdapter: SomePort,
) { ... }
}
In both scenarios, Automock confronts a dependency resolution challenge. Because it acts at runtime and relies on given identifiers to find dependencies, it may fail to distinguish between numerous implementations of the same abstract class. As a result, attempting to resolve SomePort
using unitRef.get(SomePort)
may result in ambiguity.
This problem is similar to one addressed earlier (#67), emphasizing the significance of finding clear and unambiguous means to indicate dependencies, which is why I mentioned the string (or symbol identifiers).
I hope this clarifies everything!
@omermorad I appreciate your opinion. I've never thought about multiple implementations of abstract classes. I gotta bring back the proposal when I can prevent these potential problems. Thanks :)