-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Resolving constructor arguments for derived class includes bound arguments from base classes #1565
Comments
@raymondfeng I'm not too familiar with the intended behavior of |
@mgabeler-lee-6rs I'll investigate. |
This is tricky. We have two cases:
class SubClass extends BaseClass {
constructor() {
super('sub');
}
}
class SubClass extends BaseClass {
} For case 1, we want to ignore injection from the base constructor. But for case 2, we would like to honor injection from the base constructor. Now the challenge is to differentiate the two cases and that seems to be hard. Any ideas? |
If nothing else, perhaps a decorator on the derived class constructor describing the policy for argument injections from the base class(es)? At least |
@mgabeler-lee-6rs Can you verify #1596? |
I'll try to do that next week, yes. |
Description / Steps to reproduce
When a class is being instantiated by a binding, the computed (injected) arguments for the constructor will end up including all such arguments for any base class constructors too.
The crux of this issue seems to be at https://github.com/strongloop/loopback-next/blob/master/packages/context/src/resolver.ts#L57 and https://github.com/strongloop/loopback-next/blob/master/packages/context/src/resolver.ts#L156 --
resolveInjectedArguments
does not pass options down todescribeInjectedArguments
and thus toMetadataInspector.getAllParameterMetadata
to tell it to only include 'own' parameters.As a result, you cannot really change constructor arguments in a derived class, or from the alternate perspective, you cannot put
@inject
on any constructor arguments of a base class.Current Behavior
This will fail to create an instance of
Derived
for two reasons:@inject
. If this binding is not present, it will throw an error, even though it's not needed.@inject
it will try to pass it toDerived
's constructor, which is wrong. It would get even more wrong ifDerived
had an argument of its own -- then it would be passing a completely wrong value.Expected Behavior
Injection of constructor arguments should only be for the actual constructor being invoked, and should not include arguments for base class constructors.
The text was updated successfully, but these errors were encountered: