-
Notifications
You must be signed in to change notification settings - Fork 27.4k
feat($compile): add one-way collection bindings #16553
Conversation
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.
One minor comment. LGTM otherwise.
test/ng/compileSpec.js
Outdated
|
||
expect($rootScope.collection[2]).toEqual(newItem); | ||
})); | ||
}); |
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.
Missing a test to verify the $watchCollection
part. (AFAICT, these tests would have passed even if *
was ignored.)
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.
I think the first test verifies it, and this test is just wrong/not-applicable (you can tell I copied them both from the =*
tests...).
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.
Is it worth testing the $watchCollection
is being used rather than a deep $watch
?
1380a9a
to
299e137
Compare
Updated the tests. Added the one @petebacondarwin suggested, removed the one @gkalpak pointed out was pointless, and added 2 which cause |
I wonder if |
They shouldn't be the same. |
So you'd think... but it only does |
No point in deprecating anything now @jbedard :-) |
😱 😱 😱 Sounds like a bug in |
@petebacondarwin I was thinking it would be to encourage using @gkalpak yeah... unless I'm missing something? |
There's an eslint error in compile.js (doublequotes) |
Fixed |
Implements #14039