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

Removing part of transclude destroys transclusion scope. #5703

Closed
yarontt opened this issue Jan 9, 2014 · 10 comments
Closed

Removing part of transclude destroys transclusion scope. #5703

yarontt opened this issue Jan 9, 2014 · 10 comments

Comments

@yarontt
Copy link

yarontt commented Jan 9, 2014

Hi,

I have a directive which has an isolated scope, template and transclusion. I use the transclude function in the directive's link (as expected the transclusion is bound to the parent scope, not the isolated one) to clone the template and add it to the dom. The transclusion contains several elements.

As part of the directive I sometimes remove a single element that was part of the transcluded content from the dom. This triggers $destroy on the element which destroys the transclusion scope (although there are still other elements that use it).
The $destroy binding is done in createBoundTranscludeFn().

Unless I'm missing something this seems like a bug to me.

http://jsfiddle.net/yarontt/rtwgT/

@caitp
Copy link
Contributor

caitp commented Jan 9, 2014

It doesn't seem to break when you remove a part of your template, only when you remove one of the transcluded elements. Hm

@yarontt
Copy link
Author

yarontt commented Jan 9, 2014

Ah... right.
Sorry, I guess I got mixed up when writing the issue. I'll edit to reflect the correct issue.

@ghost ghost assigned tbosch Jan 10, 2014
@tbosch
Copy link
Contributor

tbosch commented Jan 10, 2014

Would be really great to have more info about your use case in a plunker/fiddle.

@caitp
Copy link
Contributor

caitp commented Jan 10, 2014

it's not really clear what the use case is, but it seems unexpected that you get that "disconnect" effect just for removing one of the transcluded children.

@caitp
Copy link
Contributor

caitp commented Jan 10, 2014

You know what this is, it's that we're creating a new scope (

transcludedScope = scope.$new();
) due to not passing in a scope to the transclude function (
cloneAttachFn = scope;
) --- When the element gets destroy'd, so does its scope because it has no relatives (I assume, it's not totally obvious, :z how that cleanup works).

But yeah, the $rootScope continues to exist, but the bindings weren't actually referencing the $rootScope at all, but a child scope of it. This is made a bit more evident with this repro http://plnkr.co/edit/anGizYY7vXAoPvudc8bD?p=preview.

Anyways, I guess this is actually the expected behaviour, although it still totally weirds me out that we create a new scope by default there.

It's sort of related to #5489 (because ngTransclude also calls the controllersBoundTransclude with just the attach function

controller.$transclude(function(clone) {
)

Honestly, we should just throw a minErr instead (and of course, pass a proper scope to ng-transclude's controllersBoundTransclude)

I think I can sleep now ㄱ______ㄱ

@yarontt
Copy link
Author

yarontt commented Jan 10, 2014

Ah... there's a jsfiddle up there in the issue description.
Just click the "click me" and see that the binding is destroyed.

I think there's no need to create a new scope. Or at least have some something place that destroys it only if all of its elements are destroyed.

@caitp
Copy link
Contributor

caitp commented Jan 10, 2014

It only creates a new scope because you aren't explicitly passing a scope to the transclude function. So in 1.2.1 you have a workaround for this, but not if you don't call the transclude function with a scope as the first argument.

@yarontt
Copy link
Author

yarontt commented Jan 11, 2014

Yes, I've figured that out already and am using this workaround for now.

Still seems like a bug to me (took me a long time to figure out why my code wasn't working as I expected).

mishoo added a commit to kendo-labs/angular-kendo that referenced this issue Apr 16, 2014
The workaround to prevent issue #90 is revealed here, after
fair amount of googling:

    angular/angular.js#5703

Since Angular 1.2.1 we can pass $scope as first argument to
$transclude, to prevent it from creating a new scope.

We now require Angular >= 1.2.1.
@btford btford removed the gh: issue label Aug 20, 2014
@dlongley
Copy link
Contributor

dlongley commented Oct 2, 2014

Sounds like this may have been fixed by #9281.

@Narretz
Copy link
Contributor

Narretz commented Oct 19, 2014

@dlongley Good catch!

See http://jsfiddle.net/rtwgT/6/

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

No branches or pull requests

6 participants