fix($inject): enhance annotate function to work with inheritance

[rose-m]

The annotate function now respects prototypical (ES5) or class (ES6)
inheritance so that you can use these concepts for controllers
without issues. Test cases to ensure proper functionality included.

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
It changes how the $inject information is retrieved from a function/class in order to work with (prototypical) inheritance.

What is the current behavior? (You can also link to an open issue here)
When using ES6 classes and the super-class is annotated first, annotating the sub-class will always return the information of the super-class. If the sub-class requires different injections this will not work.

What is the new behavior (if this is a feature change)?
Now the annotate function respects an existing inheritance chain and computes the $inject array correctly.

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

• The commit message follows our guidelines
• Fix/Feature: Docs have been added/updated (no changes required)
• Fix/Feature: Tests have been added; existing tests pass

Other information:
No

[petebacondarwin]

Hi @rose-m. Thanks for submitting this PR. This is an area we have had lots of discussion about in the past.
Can you take a look at this comment in particular #15536 (comment).
I think it outlines the various problems that we face with supporting inheritance with AngularJS services.
If you feel that your PR does not fall foul of the discussions raised in that issue, then can you provide clarification here (and obviously a whole raft of tests that demonstrate this - especially involving native ES6 classes).

[rose-m]

Thanks @petebacondarwin for your comment - I hope I understood the discussion and will outline my approach.

Let's take a look at the following scenarios:

Inheritance with overridden constructor with own parameters:

class Base {
   constructor($serviceA) {}
}
class Child extends Base {
   constructor($serviceB, $serviceA) { super($serviceA); }
}

My changes to the annotate function will result in the following behavior:

1. When Child is annotated I check whether it hasOwnProperty('$inject') - which it won't have if it has not yet been annotated (this prevents receiving the statically inherited $inject if set on Base).
2. Then I will check and annotate the constructor function Child which will give ['$serviceB', '$serviceA'] and store it as Child.$inject.
3. Note: Missing the super call would be a programming error, i.e. the developer is responsible for forwarding the correct parameters in this case.

Inheritance with no overridden constructor/overridden constructor without arguments:

class Base {
   constructor($serviceA) {}
}
class Child extends Base {
   // no constructor or:
   constructor() {
      super(arguments); // which will be automatically called if there is no constructor
      // ... something else
   }
}

Now the annotate function will do the following:

1. Check again whether Child hasOwnProperty('$inject') - which it won't have on the first pass.
2. Compute the annotation for Child's constructor - which now is [].
3. It handles this special case that the $inject would now be empty and goes up the prototype chain of Child to find Base.
4. It annotates Base which will give $inject = ['$serviceA']. As such I deemed it safe to assume that Child needs to use the same injection otherwise it will never be possible to forward the injected services to its super constructor. Note: This will recursively continue as long as $inject = [] and there is something left up the prototype chain.
5. Child.$inject will be set to ['$serviceA'].

I hope this outlined my approaches to the problems mentioned in the other comments. I agree that it is not possible to find out whether the constructor is overridden or not - but that is where my assumption from point 4. above comes in: I just use the super-classes annotation since I don't see it possible to forward anything (which is required) to the super class.

[gkalpak]

The approach looks reasonable, but unfortunately (as mentioned in the linked discussions) there are many different usecases and there is no way to support all of them. The good news is thaat in all cases, explicitly annotating all functions works correctly (so there is always a way to handle a particular usecase).

So, at this point, I think it makes sense to strive for (a) avoiding breaking changes and (b) aiming to make the most common usecases simple (which I think it already were we're at - could be wrong though).

Note, that we always need to take the following implementations into account:

• ES5 constructor functions.
• Native ES2015 classes.
• Transpiled ES2015 classes.
• (Automatic annotations, using something like ngAnnotate.)

Off the top of my head, the proposed solution will fail in the case where an injectable class is extending a class which is not supposed to be injectable. E.g.:

class ApiClient {
  constructor(baseUrl: string) {
    this.baseUrl = baseUrl;
  }
  ...
}

myModule.service('FooApiClient', class FooApiClient extends ApiClient {
  constructor() { super('/foo/api'); }
});

myModule.service('BarApiClient', class BarApiClient extends ApiClient {
  constructor() { super('/bar/api'); }
});

In this case, the injector will look up the prototype chain and try to inject baseUrl.

[rose-m]

Hey @gkalpak - thank you for pointing this out, definitely something I didn't recognize properly in the other discussion, sorry :( So once more my PR was just a deliberate last battle cry for help to an unsolvable problem :D I much appreciate your responses!

[petebacondarwin]

@rose-m - I'm sorry that this appears intractable. The good news is that manual annotation always works!