-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($compile): bindToController
should work without controllerAs
#15110
Conversation
Looks like we can decide between this and #15106 |
Yep, I didn't see #15106 |
Let's keep this one. It's more complete than #15106. |
Is there a way to restart Travis without pushing a new commit? |
There is 😄 |
expect($rootScope.myCtrl).toBeUndefined(); | ||
} else { | ||
// The controller identifier was added to the containing scope! | ||
// Should we allow this to happen? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? It's their code, they are allowed to do things 😃
In any case, this is not related to this PR. Remove this comment and feel free to open a new issue about it if you feel it shouldn't happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wasn't sure about it...
LGTM as long as Travis is happy (and unless someone can think of a good reason not to allow this). |
c2532c6
to
9037111
Compare
LGTM, please merge |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
fix
What is the current behavior? (You can also link to an open issue here)
see #15088
Does this PR introduce a breaking change?
no
Please check if the PR fulfills these requirements