No error when trying to initialise a class without using `injectFromBase({ extendConstructorArguments: true })`

[ChristiaanScheermeijer]

Is there an existing issue for this?

• [x] I have searched the existing issues

Current behavior

We've migrated our codebase to inversify@v7. Thank you for maintaining this library 😄

When initialising an implementation class (class Impl extends Base) that doesn't use extendConstructorArguments, the injected bindings in the super class are undefined. I'm not sure if this is a bug or intended, but I had to spent some time figuring out why this was happening.

Steps to reproduce

test.ts

import { Container, injectable, inject, injectFromBase } from 'inversify';

const container = new Container({ defaultScope: 'Request' });

@injectable()
class HelperClass {
  doSomeHelpThingy() {
    console.log('OK!');
  }
}

@injectable()
class BaseClass {
  constructor(@inject(HelperClass) readonly helper: HelperClass) {}
}

@injectable()
class ImplementationClass extends BaseClass {
  test() {
    this.helper.doSomeHelpThingy();
  }
}

@injectable()
@injectFromBase({ extendConstructorArguments: true })
class ImplementationClassWorks extends BaseClass {
  test() {
    this.helper.doSomeHelpThingy();
  }
}

container.bind(HelperClass).toSelf();
container.bind(ImplementationClass).toSelf();
container.bind(ImplementationClassWorks).toSelf();

// uncomment the following line and it will crash
//container.get(ImplementationClass).test();

container.get(ImplementationClassWorks).test();

I'm running the above using tsx test.ts

Expected behavior

For me, it would have helped a lot if Inversify threw an error in this scenario.

Possible solution

If Inversify could log a warning or throw an error that would be fantastic. Especially for the ones upgrading to v7.

Package version

7.1.0

Node.js version

22.11.0

In which operating systems have you tested?

• [x] macOS
• [ ] Windows
• [ ] Linux

Stack trace

TypeError: Cannot read properties of undefined (reading 'helpMe')
    at ImplementationClass.test (test.ts:20:17)
    at <anonymous> (test.ts:36:36)
    at Object.<anonymous> (test.ts:36:41)

Other

No response

[notaphplover]

Hey @ChristiaanScheermeijer, thank you so much for opening the issue.

We've migrated our codebase to inversify@v7. Thank you for maintaining this library 😄

You're very welcome 😃, I do appreciate you kind words.

When initialising an implementation class (class Impl extends Base) that doesn't use extendConstructorArguments, the injected bindings in the super class are undefined. I'm not sure if this is a bug or intended, but I had to spent some time figuring out why this was happening.

It's the expected behavior. I think it's explained in the docs, but maybe I should update the migration guide to link to that page, I realize how confusing can result for the user following the guide.

Expected behavior

For me, it would have helped a lot if Inversify threw an error in this scenario.

I wish we could do that, but there's no way to know whether or not there's a missing injection, but some valid injections would probably be detected as false positives in the way.

[alecgibson]

Was just building a MWE because I thought this was a bug. Took me a while to track down, too. Please add this to the v6 migration docs at the very least.

I agree this is pretty bad from a consumer point-of-view: using the wrong decorator is pretty error-prone, especially because we're already using the "wrong" decorator from our v6 setup. Would be nice if the library could help us out here to identify issues somehow.

[notaphplover]

Hey @alecgibson, yeah, but as said, there's not much we can do to check whether or not the behavior is expected or a bug.

In this specific case, the cause of the issue is a tricky implementation detail. In most js engines (like v8), a class with no constructor, even if it's extending a base class, is reported to have zero constructor arguments. This is due to the fact the default constructor in a child class is the following one:

constructor(...args) {
  super(...args);
}

Which is reported to have zero arguments. This is not clear in the spec, but it's a safe asumption in js engines.

Consider the following snippet for demo purposes:

class BaseClass {
  constructor(helper) { this.helper = helper }
}

class ImplementationClass extends BaseClass {
  test() {
    this.helper.doSomeHelpThingy();
  }
}

console.log(BaseClass.length); // returns 1
console.log(ImplementationClass.length); // returns 0

Since the class constructor has zero arguments, it's perfectly fine to have zero constructor argument injections.

So, what do we do about this?

Previous inversify versions enforce a few of these tricky error cases to be reported in exchange of providing a complex inheritance mechanism which is far away to be error prone. I considered simplicity to be the lesser evil.