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

ng-scope class is set on root DOM element linked with public linking function (returned by $compile()) even if directive does not create new scope #1633

Open
danilsomsikov opened this issue Nov 30, 2012 · 18 comments

Comments

@danilsomsikov
Copy link
Contributor

See http://jsfiddle.net/danilsomsikov/aEpkh/

Original fiddle with transclusion function http://jsfiddle.net/danilsomsikov/mMWyU/4/

@pkozlowski-opensource
Copy link
Member

@danilsomsikov this works as designed. In fact a directive can create 2 sibling scopes as explained in http://docs.angularjs.org/guide/directive (check the "Understanding Transclusion and Scopes" section).

BTW: there was a nice article published just few days back that also touches on problem of transclusion:
http://blog.omkarpatil.com/2012/11/transclude-in-angularjs.html

@petebacondarwin
Copy link
Contributor

Is this not because the transclusion itself creates a new scope?

On 30 November 2012 15:48, Danil Somsikov notifications@github.com wrote:

See http://jsfiddle.net/danilsomsikov/mMWyU/4/


Reply to this email directly or view it on GitHubhttps://github.com//issues/1633.

@danilsomsikov
Copy link
Contributor Author

But as you can see in fiddle there's only one scope created, however there're two elements having css class ng-scope.

@pkozlowski-opensource
Copy link
Member

@danilsomsikov oh, sorry, didn't notice that both scopes got the same ID, I was sure that we are talking about this case: http://jsfiddle.net/GqqmS/4/

Anyway, I saw already an issue opened for those spans: #1293
There was a thread about is well: https://groups.google.com/forum/?fromgroups=#!msg/angular/2rPL2lqdCho/QU37wXUPyxoJ

Comments from Misko were:

You can thank microsoft for the spans. :-) So in IE we have this issue that we can't attach data to text content. We solve this by wrapping the text in spans. And that is what you see. Simply get rid of whitespace in your markup and spans will disappear with it. This is a bug, so you should file it, although I am not sure we can fix it without breaking IE.

On a related note, i think maybe you should do it differently. Try using the compile function to manually change the DOM structure. The template property is a short hand for compile and it looks like you have reached the end of what it can do.

So I guess it is the best to comment on your case in #1293 as this one seems to be duplicate of #1293.

@danilsomsikov
Copy link
Contributor Author

Spans are not the case as well. I don't know why did I put it to the sample code.
The problems persist without any spans.
See http://jsfiddle.net/danilsomsikov/mMWyU/5/
Directive element has ng-scope class on it, which it should not have.

@pkozlowski-opensource
Copy link
Member

@danilsomsikov Danil, thnx for providing more info, this starts to look interesting now. So this is not a new transcusion scope nor additional spans and I can't explain why this scope class is added without digging more into the code.

Re-opening this one, probably people more familiar with $compile should look into it.

@danilsomsikov
Copy link
Contributor Author

The class is added at this line.

@petebacondarwin
Copy link
Contributor

ng-scope is also added in applyDirectivesToNode() on line 558: https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L558

@danilsomsikov
Copy link
Contributor Author

Yes, however it is not the case in the issue I report.

@petebacondarwin
Copy link
Contributor

Yes you are right!

So at https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L585 you can see that the transclude function is created by a call to compile(). It is this call to compile that applies the ng-scope.

Now I guess the assumption is that if you are transcluding then you are going to create a new child scope for the transclusion, i.e. scope.$new(...). You are not doing this. You are just passing the directive's original scope into the transclusion function.

But then you are not really getting much use out of transclude in this case - you could just let the compiler pass down to the child elements itself.

I guess that you real use case is doing something more clever. But if it is then you should probably be creating a new scope and then being responsible for destroying it later, if necessary.

@petebacondarwin
Copy link
Contributor

@danilsomsikov
Copy link
Contributor Author

It turns that public linking function (the one returned by $compile(...)) is setting ng-scope CSS class and $scope data even if element does not create new scope.

@btford btford closed this as completed Aug 24, 2013
@btford
Copy link
Contributor

btford commented Aug 24, 2013

As part of our effort to clean out old issues, this issue is being automatically closed since it has been inactivite for over two months.

Please try the newest versions of Angular (1.0.8 and 1.2.0-rc.1), and if the issue persists, comment below so we can discuss it.

Thanks!

@danilsomsikov
Copy link
Contributor Author

The issue hasn't been fixed in either version.
See http://jsfiddle.net/aEpkh/3/ for 1.2.0-rc.1 and http://jsfiddle.net/aEpkh/2/ for 1.0.8

@btford btford reopened this Aug 28, 2013
@tb01923
Copy link

tb01923 commented Sep 23, 2013

I think I am experiencing a similar issue Fiddle: http://jsfiddle.net/U5UxX/

@petebacondarwin
Copy link
Contributor

Even slimmer version of example against "snapshot": http://jsfiddle.net/zPqPR/

@petebacondarwin
Copy link
Contributor

The problem is that $compile isn't to know whether you passed in a new scope or reused one from elsewhere. It currently assumes that one would never call it with a reused scope.
@danilsomsikov - What is your use case for not creating a new child scope when calling $compile?

@danilsomsikov
Copy link
Contributor Author

If $compile doesn't know what kind of scope it gets, why would it set css class? I don't have particular use case, I found it when I was reading source code to study how compilation works.

@jeffbcross jeffbcross added this to the Purgatory milestone Feb 26, 2014
@btford btford removed the gh: issue label Aug 20, 2014
@Narretz Narretz modified the milestones: Ice Box, Purgatory Oct 31, 2015
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

7 participants