Feature: provide automatic annotation for derived classes

[hitmands]

Currently:

// @ngInject
// outputs: Foo.$inject = ['$q']
class Foo {
  constructor($q) {
    this.$q = $q;
  }
}

// @ngInject
class Baz extends Foo {
  logQ() { console.log(this.$q); } // undefined ?
}

angular
  .module('test', [])
  .service('Foo', Foo)
  .service('Baz', Baz)
;

Expected

It would be helpful if also Baz will inherit dependencies from Foo.

StackOverflow issue: AngularJS: Inherited dependencies need to be duplicated?

[schmod]

The ES2015 and ES5 transpiled output both work fine when I try them.

---

If Baz has its own constructor with different arguments than Foo, we should be able to handle it fine as long as both Foo and Baz have ngInject annotations, and you preserve the ordering of the constructor arguments (ugh). I'm hesitant to add support for rest parameters (as you describe in the Stack Overflow issue), because I'm worried about edge-cases (and it would be rather difficult to implement)

[hitmands]

Yes, I totally agree with your point of view...

it could be an option to live this issue open? Someone could end up with the right solution...

We basically need to recursively build the $inject static property.

// @ngInject
class A {
  constructor($1) {}
}
expect(A.$inject).toBe(['$1']); // passed

// @ngInject
class B extends A {
  constructor(/* $1, */ $2) {}
}

expect(B.$inject).toBe(['$1', '$2']); // instead got ['$2']

---

we could also consider to automatically inject $1 into B constructor...

[schmod]

The problem is that we can't guarantee the ordering of arguments is always going to line up between B and A, or that rest parameters are necessarily always going to be passed directly into super().

Like I said, I think the only "safe" thing that I can recommend is for B to directly declare all of its injectables as well as those of its superclasses in its constructor. Any calls to super() need to use normal JS semantics (ie. not use Angular's DI), and list out the constructor arguments in order.

This will work:

class A {
  constructor($1) {}
}
expect(A.$inject).toBe(['$1']); // passed

// @ngInject
class B extends A {
  constructor($1, $2) {
    super($1);
  }
}
expect(A.$inject).toBe(['$1','$2']); // passed

---

There is a convoluted path by which we could theoretically make this work by replacing calls to super() with $injector.invoke() (which would in turn need to reflect upon B.$inject to build up a locals object), but it feels exceptionally hacky.

// @ngInject
class B extends A {
  constructor($1, $2) {
    const locals = {};
    B.$inject.forEach((local, i) => locals[local] = arguments[i]);
    $injector.invoke(A, this, locals);
  }
}
B.$inject=['$1','$2'];

Note: I'm not even sure that this would work.

---

I'm only comfortable adding transformations that won't add significant weight to the code, won't add new features to Angular, and won't introduce significant hidden pitfalls. This feels like it does all 3.

Angular 1.x's DI mechanism was not designed with classical inheritance in mind, and I'm hesitant to add any features to this plugin that attempt to cover up Angular's own shortcomings (as any workarounds are likely to be fragile, incomplete, or confusing).

This plugin is intended to ensure that idiomatic Angular 1.x code can survive minification while remaining DRY. That's it. We're not going to add features that extend/augment the existing DI mechanism, nor are we going to add features or transformations that break compatibility with the existing ecosystem. Unminified code should always work without being processed by this plugin.

If the Angular 1.x maintainers can explain how the existing DI framework should be used with derived classes (or add features that allow it to be done elegantly), I'll gladly update this library to ensure that code written with those patterns can survive minification. Anything else is probably going to remain out of scope.

[hitmands]

I completely agree. Considering the scope of this plugin, this feature is definitely out-of-scope.

[schmod]

Thanks for taking the time to open an issue though! If you can think of a straightforward way that we can add this functionality (or want to open up a dialogue with the Angular folks), I'm happy to consider it.

Similarly, if you want to write your own Babel plugin (a suggestion I make in response to a lot of feature requests here), I'm happy to point you in the right direction. Despite the relatively poor state of the documentation, it's less intimidating than you'd think!

[zuzusik]

I recently encountered this issue. Solved it with adding following code to the base class:

Note: this approach would only work if base class gets auto-annotated - so it either has to be used in angular.module('<blah>').<method>() call or should have explicit // @ngInject comment.

  static set $inject(tokens) {
    if (tokens.length) {
      this._$inject = tokens;
    }
  }
  static get $inject() {
    return this._$inject;
  }

  static _$inject;

The idea here is that when derived class doesn't have own constructor, automatic annotation would set [] as $inject to it, and so the class would use parent's $inject as setter would skip setting _$inject. For derived classes with own constructor, automated annotation would assign $inject as not empty array and so setter would set _$inject and class will have own $inject this way.