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

fix($compile): render nested transclusion at the root of a template #8933

Closed
wants to merge 1 commit into from

Conversation

petebacondarwin
Copy link
Member

Closes #8914
Closes #8925

@caitp
Copy link
Contributor

caitp commented Sep 4, 2014

Looks good now

@petebacondarwin
Copy link
Member Author

Merged.


$compileProvider.directive('inner', valueFn({
transclude: true,
template: '<div ng-transclude></div>'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to remove the ng-transclude directive and the test will still pass (transclusion still gets linked and appended) - just because there is no child element. That is wrong.

@vojtajina
Copy link
Contributor

@jeffbcross @petebacondarwin This fix is wrong and we should revert it.

It links the transclusion when there is no child element instead of when ng-transclude directive is present. See my comments in test/ng/compileSpec.js.

Fixing this issue is hard. In other cases (when ng-transclude is on some child element) we use that element as a container - we pass that element to children as a transclusion and then, ng-transclude will mutate this element, without worrying that some other transclusion moved (transcluded) the container element somewhere else. In this cases, there is no container to mutate.

At the time when we are creating the transclusion (unbound transclusion, during compilation) we don't have the information that there is ng-transclude present which will append some content (because ng-transclude is just an arbitrary directive that asks for $transclude and does append the content later, during linking).

This fix is basically treating the case "no children element" as "it has ng-transclude". I don't think that's correct.

@petebacondarwin
Copy link
Member Author

Ok let's revert as it was an unusual corner case anyway. Sorry to break things

@petebacondarwin petebacondarwin deleted the issue-8914 branch December 17, 2015 12:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants