-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Conversation
Conflicts:
packages/ember-runtime/lib/system/each_proxy.js
packages/ember-runtime/tests/mixins/array_test.js
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 this._keys ever legitimately contain prototype extensions? If not, maybe we should make it an EmptyObject. This would allow us to also skip the bellow !keys.hasOwnProperty(key)
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.
a here it is: https://github.com/emberjs/ember.js/blob/fix-each-beta/packages/ember-runtime/lib/system/each_proxy.js#L79
ya this should be one of those EmptyObjects and we can take the fast-path for x in y without HOP.
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.
It is a empty, this is beta though
|
i believe this needs a deprecation in 1.13.x (and it is worth any tiny pain users may have with an additional deprecation. |
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.
funky whitespace
|
I just want to make sure I'm clear on this cause the wording of the description is a bit confusing. Say I have 40;
});">isOvertime: Ember.computed('shifts.@each.hours', function() {
var total = this.get('shifts').reduce(function (sum, shift) { return sum + shift.get('hours'); }, 0); return total > 40; }); Would |
|
@raytiley ok, I do need to make it more clear, this is only targeting where people used |
|
gotcha... so doing |
|
Merging this into beta. I am working on some deprecations specifically for stable that will ensure |
|
This change should be probably mentioned clearly in the next beta release notes. There must be few people like me who have made the switch to 2.0 beta already and won't see the deprecation. Still a really minor thing, those who use the very latest versions probably follow PRs too. |
|
@rwjblue based on ^ should we leave some hints/warnings/asserts/errors in 2.x? Might be a common thing, especially as people who are knew to the concept. We can likely catch the mistake extremely early and instruct the correct usage. |
|
@stefanpenner / @ilkkao - Helpful assertion added in #12019. |
|
cool, thanks. |