Fix $compile (remove templateUrl directive splitting)

[vojtajina]

How did compiling a templateUrl (async) directive with replace:true work before this commit?

1. apply all directives with higher priority than the templateUrl directive
2. partially apply the templateUrl directive (create beforeTemplateNodeLinkFn)
3. fetch the template
4. apply second part of the templateUrl directive on the fetched template
   (afterTemplateNodeLinkFn)

That is, the templateUrl directive is basically split into two parts (two nodeLinkFn functions),
which has to be both applied.

Normally we compose linking functions (nodeLinkFn) using continuation - calling the linking
function of a parent element, passing the linking function of the child elements as an argument. The
parent linking function then does:

1. execute its pre-link functions
2. call the child elements linking function (traverse)
3. execute its post-link functions

Now, we have two linking functions for the same DOM element level (because the templateUrl directive
has been split).

There has been multiple issues because of the order of these two linking functions (creating
controller before setting up scope locals, running linking functions before instantiating
controller, etc.). It is easy to fix one use case, but it breaks some other use case. It is hard to
decide what is the "correct" order of these two linking functions as they are essentially on the
same level.

Running them side-by-side screws up pre/post linking functions for the high priority directives
(those executed before the templateUrl directive). It runs post-linking functions before traversing:

beforeTemplateNodeLinkFn(null); // do not travers
afterTemplateNodeLinkFn(afterTemplateChildLinkFn);

Composing them (in any order) screws up the order of post-linking functions. We could fix this by
having post-linking functions to execute in reverse order (from the lowest priority to the highest)
which might actually make a sense.

My solution is to remove this splitting. This commit removes the beforeTemplateNodeLinkFn. The
first run (before we have the template) only schedules fetching the template. The rest (creating
scope locals, instantiating a controller, linking functions, etc) is done when processing the
directive again (in the context of the already fetched template; this is the cloned
derivedSyncDirective).

We still need to pass-through the linking functions of the higher priority directives (those
executed before the templateUrl directive), that's why I added preLinkFns and postLinkFns
arguments to applyDirectivesToNode.

This also changes the "$compile transclude should make the result of a transclusion available to the
parent directive in post- linking phase (templateUrl)" unit test. It was testing that a parent
directive can see the content of transclusion in its pre-link function. That is IMHO wrong (as the
ngTransclude directive inserts the translusion in its linking function). This test was only passing because of
c173ca4, which changed the behavior of the compiler to traverse
before executing the parent linking function. That was wrong and also caused the #3792 issue, which
this change fixes.

Closes #3792

[mary-poppins]

Don't mind me.

[petebacondarwin]

This idea seems very sound.

What is the rationale behind changing the name of replaceDirective to ignoreDirective? Also there seem to be quite a few documented functions that take this ignoreDirective as a parameter but don't have an @param section in their documentation for it.

[IgorMinar]

I don't have the mental brain power to review this properly now, but my first gut feeling is that this change will delay the instantiation of controllers until the templates are available, which can significantly delay the instantiation and with it the whole app.

Also it sounds like the order of events is different for a directive with template vs templateUrl vs templateUrl + replace

[vojtajina]

@petebacondarwin see my response to misko - I don't care about this name change that much. As per missing docs comments, good point, I can add them.

@IgorMinar Yep, the controller is instantiated once we have the template, but I believe that's how it always was. Controllers are instantiated in nodeLinkFn (which is linking function) - you can't run linking function before you have the template. Also the controller can ask for $element, which you don't have before you fetch the template.

Yep, the order of events is not entirely the same (template vs. templateUrl), but I doubt we can make it entirely the same without making even the template directives asynchronous. I believe that order of the events regard the async directive itself (link fn, instantiation of the controller, initialization of scope locals, etc.) is actually the same as if it was just a sync template directive.

That said, the compiler code is probably the most complex code I've ever seen and therefore anything I said might be wrong. So, if you have any concerns, write a failing test.

I tried (three times) to make major changes in how the compiler internally works in order to make it "cleaner", but I always failed to make it work with all the different corner cases.

[vojtajina]

@IgorMinar let me know, if you want me to do some more work on this, but I believe this fix should go into the next release, as the current state is seriously broken (it links children directives first; when using templateUrl).

[IgorMinar]

@vojtajina the test that you removed still passes as is with your fix. I found it weird that it's still the case.

[trask]

I unknowingly submitted a fix for the same underlying issue in #4193 (thanks @rodyhaddad for catching this!). My fix doesn't seem as good, but has a couple of good tests you may find useful here.