-
Notifications
You must be signed in to change notification settings - Fork 27.3k
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 😄 |
test/ng/compileSpec.js
Outdated
| 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