fix($compile): throw error when requesting new and isolate scopes (async) #12217
Conversation
gkalpak
force-pushed
the
fix-$compile-throw-multidir
branch
from
0ad06c6 to
0d92eb6
Compare
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
|
We should really fix this. This might be a breaking change for some apps, although it's technically a fix. If you didn't know you were not supposed to use it this way, you'll simply experience your app breaking. |
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
force-pushed
the
fix-$compile-throw-multidir
branch
from
0d92eb6 to
c715553
Compare
|
Reviewed 1 of 2 files at r1. Comments from the review on Reviewable.io |
|
Reviewed 1 of 2 files at r1. Comments from the review on Reviewable.io |
all files reviewed at latest revision, all discussions resolved, all commit checks successful.
|
LGTM Review status: Comments from the review on Reviewable.io |
|
I don't think it would be a breaking change, since it is invalid to have new scope and isolate scope together. Review status: Comments from the review on Reviewable.io |
gkalpak
changed the title
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
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 #12215