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

Isolate scope weird behavior based on directive's name. #12215

Closed
tosdan opened this issue Jun 26, 2015 · 2 comments
Closed

Isolate scope weird behavior based on directive's name. #12215

tosdan opened this issue Jun 26, 2015 · 2 comments

Comments

@tosdan
Copy link

tosdan commented Jun 26, 2015

I have 2 directives on the same element. One has isolate scope (scope: {}), the other one inherits scope from parent (scope: true). Angular version 1.3.16 (and even 1.4).

This should throw an exception, but it doesn't if my directive's name starts with letter "o" or any following letter in the alphabet.

See for your self in the following plunk by simply changing the directive's name first letter:
http://plnkr.co/edit/oL1WLw

@gkalpak
Copy link
Member

gkalpak commented Jun 26, 2015

Without looking deep into, it has clearly to do with directive priorities.
Since all involved directives have equals (=default) priority, their relative priority is determined by alphabetic order (so the o is coming from otherDirective - e.g. if you rename it to zOtherDirective then both myDirective and oyDirective will throw).

The order of processing its directive changes the outcome, because (for some reason) the check about isolate+new scope is skipped when there is a templateUrl with the following comment:

//skip the check for directives with async templates, we'll check the derived sync
// directive when the template arrives

I am not sure if we ever do check when the template arrives (or if we check properly).

@gkalpak
Copy link
Member

gkalpak commented Jun 26, 2015

A little more context (Angular internals stuff - for source-viewer only :P):

We indeed fail to properly check, because the newScopeDirective is not preserved in the previousCompileContext when calling compileTemplateUrl() (compile.js#L1820).

As a result, we (accidentally ?) allow directives that request a new normal scope and a new isolate scope on the same element (as long as the isolate-scope-requesting directive specifies templateUrl).

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jun 26, 2015
While directives are not allowed to request both a new (normal) and an
isolate scope on the same element, the relevant check was not performed
correctly when the directive requesting the isolate scope had a lower
priority and specified a `templateUrl`.
In that case, the check was deferred until the template was fetched, but
the info about other directives requesting a new (normal) scope was not
properly preserved (in `previousCompileContext`).

This commit fixes this, so it now an error is thrown (as expected).

Fixes angular#12215
gkalpak added a commit to gkalpak/angular.js that referenced this issue Jun 26, 2015
While directives are not allowed to request both a new (normal) and an
isolate scope on the same element, the relevant check was not performed
correctly when the directive requesting the isolate scope had a lower
priority and specified a `templateUrl`.
In that case, the check was deferred until the template was fetched, but
the info about other directives requesting a new (normal) scope was not
properly preserved (in `previousCompileContext`).

This commit fixes this, so now an error is thrown (as expected).

Fixes angular#12215
Closes angular#12217
gkalpak added a commit to gkalpak/angular.js that referenced this issue Jun 26, 2015
While directives are not allowed to request both a new (normal) and an
isolate scope on the same element, the relevant check was not performed
correctly when the directive requesting the isolate scope had a lower
priority and specified a `templateUrl`.
In that case, the check was deferred until the template was fetched, but
the info about other directives requesting a new (normal) scope was not
properly preserved (in `previousCompileContext`).

This commit fixes this, so now an error is thrown (as expected).

Fixes angular#12215
Closes angular#12217
netman92 pushed a commit to netman92/angular.js that referenced this issue Aug 8, 2015
While directives are not allowed to request both a new (normal) and an
isolate scope on the same element, the relevant check was not performed
correctly when the directive requesting the isolate scope had a lower
priority and specified a `templateUrl`.
In that case, the check was deferred until the template was fetched, but
the info about other directives requesting a new (normal) scope was not
properly preserved (in `previousCompileContext`).

This commit fixes this, so now an error is thrown (as expected).

Fixes angular#12215
Closes angular#12217
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.

2 participants