Inject parent components into child components

[eXaminator]

This implements #98, but it only works for transcluded children. Direct children can't retrieve their parents this way because you get a circular dependency between the the parent (which needs the child for its directives option) and the child (which needs its parent for the @Inject decorator).

[eXaminator]

I just thought about how "optional" parents could be handled (like require in angular 1 allows) and came to the conclusion that I could drop the check on $injector if there is a service of the same name and just do "components first". This would allow to manually add a service/value/constant that uses the component class as name which should then be used as fallback wenn a component requires another component but doesn't find it.

Example: I could provide an empty object as value for Parent and if I have a Child that requires a Parent to be present but doesn't have one it would instead receive the empty Object as fallback. So the user could supply whatever fallback he would like.

Of course this doesn't work on a "per component" basis, but I'd say this could be pretty much how angular 2 would handle it.

What do you say? @timkindberg

[timkindberg]

Talked with @MikeRyan52 about this.

• On the issue of a not found controller, throw an error just as you already are.
• On the issue of finding a transclusion. It should inject a parent regardless of transclusion. So let's add another test for non-transcluded scenario and see if it works. Per the cyclic dependency, @MikeRyan52 had this to say:

That's actually a CommonJS issue. I think ES6 has a notion of hot modules, which means the cyclic dependency would resolve naturally Though I understand the issue I think Angular 2 has a futureReference helper somewhere Look for forwardRef https://angular.io/docs/ts/latest/api/core/forwardRef-function.html

If it works, great! If not, see if it would be too hard to do a lightweight forwardRef implementation. If it's too crazy then we can limit this to transclusion scenarios only for now and deal with non-transcluded in the future.

Again thanks!!!!

[timkindberg]

Also let's squash all the commits before we merge it. Make the commit against feat(Inject): find and inject parent controllers as locals

[timkindberg]

per your comment: https://github.com/ngUpgraders/ng-forward/pull/112#issuecomment-162354595

I don't think we need to provide a way to provide a fallback. I'm not sure Angular 2 has that beyond what you can already do with using provide to set up a service/component of the same name so it clobbers the original.

I downloaded the real Angular 2 so I could see how it behaves in various scenarios, here are the results:

• Transclusion scenario (e.g. Tabs and Tab used by a 3rd component using transclusion):
  • When parent is present: success
  • When parent is missing: Throws 'provider missing' error
  • When parent is missing, but inject was marked as @Optional(): Works just fine, but parent param comes in as null. So a check is necessary if(tabs){}. My constructor of the child looks like this: constructor(@Optional() tabs: Tabs) {}.
• Non-transclusion scenario (e.g. when Parent uses Child in its own view):
  • When parent is present: Throws 'EXCEPTION: Cannot resolve all parameters for Tab(undefined). Make sure they all have valid type or annotations'
  • When parent is present, and I use forwardRef like so: constructor(@Inject(forwardRef(() => Tabs)) tabs) {}: success
  • When parent is missing, and I use @Optional and forwardRef: constructor(@Optional() @Inject(forwardRef(() => Tabs)) tabs) {}: success

So again I'm not sure how much farther you'd like to take this. What you've done so far has value and can stand on it's own if you'd like to just wrap it up and get this merged. But there's all the details if you want to keep going :)

@Optional()

I imagine we could make our @Optional decorator that just adds a flag to the class; would be pretty simple. Then we could use it like so @Inject(@Optional() Parent). You'd have to reflect that it is optional somehow in the inject string. That might be weird. I'm wondering if we should reconsider using require instead of climbing the DOM looking for controllers... we could use the componentHooks.extendDDO method to add ddo.require as needed. I'm not sure...

forwardRef

This one I'm not so sure about how to implement. @Inject would need to be extended to allow for function params. Those function params would still need to be able to be decorated by @Optional. This one seems like a larger change to me... like we might have to alter our injection code to allow for just-in-time resolution of the Type.

Sorry for the long post :/

[eXaminator]

I'll have a look at the refactorings tonight.

As for the other matters:

forwardRef

I have an idea for this: We could have @Inject() check if it got a function as injectable and if it does, keep it intact in the $inject array. We could then decorate (as in 'angular.js decorator') the $injector service and make it resolve functions before it runs its methods. We'd also have to update code where we manually work with the injects (like my above code). This is a very rough idea, so I'm not sure if that would actually work. We could limit forwardRef to components and only update the above code, but I think that would be rather limiting.

I could give it a try if you'd like.

Optional

This one is hard. Your @Optional proposal would only work in TypeScript, but as it is discussed in https://github.com/angular/angular/issues/5449 I might have to live with the thought that I have to either get rid of ESnext altogether (and work with ES5 syntax) or I upgrade to typescript (which is probably what I'll do). Never the less I'd rather not implement that right now as I don't feel I have enough knowledge about typescript right now to implement a completely new parameter decorator.

[eXaminator]

So I fixed the location where I add the callback, but I got into something of a dead end regarding the forwardRef as it wasn't as easy as I hoped it would be. @Inject does more then I first expected which makes this approach harder. I'll think about it a while longer and see if I can come up with something.

Though I wonder if it's worth it if @Inject might be changed to an attribute decorator anyway.

[eXaminator]

But another thought: Maybe we should start searching on the same element as long as we are not requesting the same instance? Do you know how angular 2 handles this?

[timkindberg]

I recommend cloning the ng2-play repo so you can try some things out. It's helped me quite a bit. If you don't have time I can check on that for you later.

[timkindberg]

I tried putting two directives (not components) on the same div and have one try to inject the other. It got the unknown provider error. Is this what you were wondering about?

<div tabs tab></div>

Also if I put one directive on a parent element it didn't work either (with the nested directive asking for the one on the parent):

<div tabs>
    <div tab></div>
</div>

[eXaminator]

Mh, so I guess it's not possible to require directives on the same element at all? That makes a certain pattern I was thinking of impossible, but I guess there are other ways. I will check out the repo you mentioned, but I don't think I'll get to it any time soon.

And as the whole inject API might change I'd rather not invest more time right now into this matter. So maybe it should be merged. I wasn't able to find a simple solution for forwardRef anyway. I'll see if I can squash the commits tonight.

[timkindberg]

Yeah that's how it appears... kind of weird. But Pascal is saying that its possible in Angular 2 and the default in 2 is to look on the current element first and then travel up the DOM. He said there is another decorator (@SkipSelf) that makes DI start on the parent, but we don't need to do that just yet.

[timkindberg]

How about this: If you squash it I'll merge it

We'll deal with the other stuff later.

[timkindberg]

Oh we also need updates to the API.md file. Which I could do if you don't have time, but would love if you did them :)

[timkindberg]

Lastly, maybe we should just allow the same element directive injection, since Pascal is saying it is supposed to work. So can you start on the current element and write a test for that? If you just wanna be done I totally understand.

[timkindberg]

For future reference: https://angular.io/docs/ts/latest/api/core/Directive-var.html#understanding-how-injection-works

[eXaminator]

Ok, I changed the logic to start searching on the current element and updated the API docs. Please review and merge if everything is ok! :)

[timkindberg]

Looks good thanks @eXaminator !!!