Make array macros work with native JS arrays #384
Conversation
addon/array/uniq-by.js
Outdated
| }); | ||
|
|
||
| return ret; | ||
| } |
There was a problem hiding this comment.
Maybe I don't get why this was necessary but emberA(array) should really have the same result.
There was a problem hiding this comment.
Nice! Simplify all the things :)
There was a problem hiding this comment.
Apparently uniqBy doesn't exist in Ember 1.13: https://travis-ci.org/kellyselden/ember-awesome-macros/jobs/270136987#L2742 😞
|
@kellyselden: would be great to get this merged |
| @@ -1,7 +1,6 @@ | |||
| import { findBy } from 'ember-awesome-macros/array'; | |||
| import { raw } from 'ember-awesome-macros'; | |||
| import EmberObject from '@ember/object'; | |||
| import { A as emberA } from '@ember/array'; | |||
There was a problem hiding this comment.
Should all these lines be reverted now?
| @@ -12,6 +12,7 @@ export default createClassComputed( | |||
| if (array === undefined || key === undefined) { | |||
| return array; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
We should probably do a array = emberA(array); here. That way we skip the polyfill for newer Ember version, and we are more consistent with the other array macros.
There was a problem hiding this comment.
I'm not sure what you mean tbh.
There was a problem hiding this comment.
I'll fix it in a subsequent PR and tag you.
|
Thanks a lot. |
The array macros (or most of them) assumed the source arrays to be
Ember.Arrays which I don't think is necessary (and breaks without prototype extensions, e.g. in FastBoot).Although CPs won't update correctly when defined on native JS arrays there's no reason why they should not compute correctly initially (and that's also what Ember's own array CP macros do).