Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Link fn of a replace+transclude directive runs before transclusion takes place #3558

Closed
aeby opened this issue Aug 13, 2013 · 24 comments
Closed

Comments

@aeby
Copy link
Contributor

aeby commented Aug 13, 2013

I'm unable to access transcluded elements in a link function. The behavior has changed since 1.1.5. See http://plnkr.co/edit/WvGAlByx1771wurPbWXA?p=preview and compare the result with the commented 1.1.5 script

app.directive('test', [function () {
  return {
    restrict: 'E',
    transclude: true,
    replace: true,
    template: '<div ng-transclude></div>',
    link: function postLink(scope, element) {
      // This should log 2 as in 1.1.5
      console.log(element.children().length);
    }
  };
}]);

element.children(); should contain the transcluded elements like the `h2' tags:

<test>
    <h2>I'm transcluded</h2>
    <h2>Me too</h2>
<test>
@bobslaede
Copy link

http://plnkr.co/edit/PFtqmLwPKX4UJbLOEGtg?p=preview

Your example was broken

@aeby
Copy link
Contributor Author

aeby commented Aug 14, 2013

@bobslaede Thanks for the fix.

@flyingmutant
Copy link

I can confirm this bug — my directives became broken after upgrade from 1.1.5 to 1.2.0-rc1.

@aeby
Copy link
Contributor Author

aeby commented Aug 14, 2013

It looks like the compile order has changed (maybe here 683fd71). If I transclude a directive that uses the same parent controller as my directive does (e.g. I wrap an <input> in a custom <formfield> directive and both require: '^form') prior to 1.2rc1 the common controller was already initialized by the transcluded directive.
I don't know if this is a bug or if this is the intended behavior.

@bobslaede
Copy link

I don't think it is intended behavior, when the dom is not "ready" for the post-link function.
I can see how it is not ready for the compile, and pre-link function.
I can't find any tests that describe that the pre and post link functions should have access to transcluded elements.

@aeby
Copy link
Contributor Author

aeby commented Aug 14, 2013

Yes I guess you are right, the dom has to be ready in the post-link function.

@bobslaede
Copy link

I'm guessing its the $scope.$evalAsync method call. There's no way to know when that is done.
I will try debug some stuff locally :)

@bobslaede
Copy link

Changing $evalAsync to just $eval makes everything available in both pre and post link. Maybe there's some performance gain here? Somebody must know :)

@aeby
Copy link
Contributor Author

aeby commented Aug 14, 2013

Do you talk about this 683fd71#L0R56 $evalAsync?

@bobslaede
Copy link

Yeah, that one

@aeby
Copy link
Contributor Author

aeby commented Aug 14, 2013

So I guess @IgorMinar should know :)

@bobslaede
Copy link

Changing $evalAsync doesn't help that bug with ng-include sadly...

@bobslaede
Copy link

I just had a chance to run the unit tests, they fail with the "fix".
$compile controller should instantiate controllers in the parent->child->baby order when nested transluction, templateUrl and replacement are in the mix FAILED

@baubie
Copy link

baubie commented Aug 28, 2013

This isn't solving the underlying problem, but I was able to get around this by wrapping my link function contents in a $timeout(). In my case, my template contains a <code ng-transclude></code> and I need to access those contents.

// Fails
link: function(scope, elm, attrs) {
    console.log(elm.find('code').text());  // Blank
}

// Works
link: function(scope, elm, attrs) {
    $timeout(function() {
        console.log(elm.find('code').text());  // Expected text
    }, 0);
}

EDIT: Just to clarify, I'm not proposing this as a solution, just a temporary work around.

@aeby aeby closed this as completed Aug 29, 2013
@barakka
Copy link

barakka commented Aug 29, 2013

Why has it been closed? Is there a fix? (The timeout trick doesn't seem to be too convincing).

@flyingmutant
Copy link

Yep — timeout is a hacky workaround indeed, please reopen the issue.

@aeby aeby reopened this Aug 29, 2013
@aeby
Copy link
Contributor Author

aeby commented Aug 29, 2013

Sorry, I didn't intend to close the issue.

bradw2k referenced this issue Sep 10, 2013
previously the translusion was appended the the ngTranslude element via
$evalAsync which makes the transluded dom unavailable to parent
post-linking functions. By appending translusion in linking phase,
post-linking functions will be able to access it.
@bradw2k
Copy link
Contributor

bradw2k commented Sep 10, 2013

I wonder if rc2 fixes it with this:
bf79bd4
Sounds right but I have not tested it myself.

@bradw2k
Copy link
Contributor

bradw2k commented Sep 11, 2013

Looks like @bobslaede 's example still fails (I updated below). Transcluded content is in the DOM during post-link when using 1.1.5, but not when using 1.2.0rc1 nor 1.2.0-rc.2:
http://plnkr.co/edit/JiphJK?p=preview

@flyingmutant
Copy link

I can confirm that the bug is still present in 1.2.0-rc2.

@ghost ghost assigned IgorMinar Sep 27, 2013
@IgorMinar
Copy link
Contributor

It's important to note that the issue affects only directives that have both replace and transclude options set to true. bf79bd4 fix the issue for directives that didn't also requested to be replaced. I'm investigating a fix for replace+transclude scenario.

@IgorMinar
Copy link
Contributor

This should get fixed by #3927

@IgorMinar
Copy link
Contributor

#3927 didn't fix this, we need a different fix. I'm working on it.

IgorMinar added a commit to IgorMinar/angular.js that referenced this issue Oct 3, 2013
previously the compile/link fns executed in this order controlled via priority:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityHigh, PostLinkPriorityMedium, PostLinkPriorityLow

This was changed to:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityLow, PostLinkPriorityMedium , PostLinkPriorityHigh

Using this order the child transclusion directive that gets replaced
onto the current element get executed correctly (see issue angular#3558),
and more generally, the order of execution of post linking function
makes more sense. The incorrect order was an oversight that has
gone unnoticed for many suns and moons.

(FYI: postLink functions are the default linking functions)

BREAKING CHANGE: the order of postLink fn is now mirror opposite of
the order in which corresponding preLinking and compile functions
execute.

Very few directives in practice rely on order of postLinking function
(unlike on the order of compile functions), so in the rare case
of this change affecting an existing directive, it might be necessary
to convert it to a preLinking function or give it negative priority
(look at the diff of this commit to see how an internal attribute
interpolation directive was adjusted).

Closes angular#3558
@jbruni
Copy link
Contributor

jbruni commented Oct 19, 2013

@IgorMinar - documentation about this "issue/fix/breaking change" says:

Very few directives in practice rely on order of postLinking function
(unlike on the order of compile functions), so in the rare case
of this change affecting an existing directive, it might be necessary
to convert it to a preLinking function or give it negative priority
(look at the diff of this commit to see how an internal attribute
interpolation directive was adjusted).

Look at my case:
rc.2 = http://plnkr.co/edit/xMKPSHvgTQxx7wWn6p0m?p=preview
rc.3 = http://plnkr.co/edit/9835vKaTxNBftfxobVQt?p=preview

So, I had this "quick & dirty" super-simple directive coupled with ngPattern working fine with rc.2, and suddenly not working anymore with rc.3.

The directive adds a $formatter and a $parser to input elements, and it is being used together with ngPattern validation. After spending quite some time to understand what was happening, i've found that: in rc.2 the ngPattern validation is applied to the formatted value, while in rc.3 the validation is applied to the parsed value. At first, I thought there was some change to ngPattern implementation; it took me a bit more of time to understand it was related to the present issue/fix/change (which seemed to me totally unrelated at a first glance).

After comprehending the issue, it became extremely easy to fix the situation. Anyway, I must question the documentation due to:

  1. Rare case? The directive is so simple, and I've built it from a similar one found at Stack Overflow... a quick search now brought me:
  2. About the proposed fix: either preLinking or setting a negative priority does not affect the behaviour at all. Now, if I set priority to a positive number (any number greater than zero) to the directive, then the behaviour is reverted to rc.2 functionality. (Easy to verify in the rc.3 Plunker linked above.)

Anyway, strictly considering my case, the rc.3 behaviour is better, since the ngPattern regexp validation is applied into the parsed value, instead of the formatted value - it means I can write the regexp to the model value - not the modified view value. So, my "fix" for rc.3 consists in change a single character in my regular expression (better than doing something to revert to the previous behaviour).

Someone else may befenit of this testimonial I've just shared right here, but maybe we can also consider enhancing this breaking change related documentation...

Thanks!

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
previously the compile/link fns executed in this order controlled via priority:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityHigh, PostLinkPriorityMedium, PostLinkPriorityLow

This was changed to:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityLow, PostLinkPriorityMedium , PostLinkPriorityHigh

Using this order the child transclusion directive that gets replaced
onto the current element get executed correctly (see issue angular#3558),
and more generally, the order of execution of post linking function
makes more sense. The incorrect order was an oversight that has
gone unnoticed for many suns and moons.

(FYI: postLink functions are the default linking functions)

BREAKING CHANGE: the order of postLink fn is now mirror opposite of
the order in which corresponding preLinking and compile functions
execute.

Very few directives in practice rely on order of postLinking function
(unlike on the order of compile functions), so in the rare case
of this change affecting an existing directive, it might be necessary
to convert it to a preLinking function or give it negative priority
(look at the diff of this commit to see how an internal attribute
interpolation directive was adjusted).

Closes angular#3558
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
previously the compile/link fns executed in this order controlled via priority:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityHigh, PostLinkPriorityMedium, PostLinkPriorityLow

This was changed to:

- CompilePriorityHigh, CompilePriorityMedium, CompilePriorityLow
- PreLinkPriorityHigh, PreLinkPriorityMedium, PreLinkPriorityLow
- link children
- PostLinkPriorityLow, PostLinkPriorityMedium , PostLinkPriorityHigh

Using this order the child transclusion directive that gets replaced
onto the current element get executed correctly (see issue angular#3558),
and more generally, the order of execution of post linking function
makes more sense. The incorrect order was an oversight that has
gone unnoticed for many suns and moons.

(FYI: postLink functions are the default linking functions)

BREAKING CHANGE: the order of postLink fn is now mirror opposite of
the order in which corresponding preLinking and compile functions
execute.

Very few directives in practice rely on order of postLinking function
(unlike on the order of compile functions), so in the rare case
of this change affecting an existing directive, it might be necessary
to convert it to a preLinking function or give it negative priority
(look at the diff of this commit to see how an internal attribute
interpolation directive was adjusted).

Closes angular#3558
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants