Array functions

[mmun]

Rendered

[Turbo87]

so the new functions are delegating to the old methods? wouldn't that mean we're duplicating the implementation (once in the new functions for native arrays and once in the old methods for extended prototype arrays)?

[mmun]

so the new functions are delegating to the old methods?

Yes, for objects that are not native arrays we must call the method.

wouldn't that mean we're duplicating the implementation (once in the new functions for native arrays and once in the old methods for extended prototype arrays)?

In the existing code, each implementer of MutableArray must supply an objectAt and a replace method. I wouldn't say that this could is "duplicated" but rather that each class has a "specialized" version of the method. This proposal is simply to shuffle around the existing code.

For example, if NativeArray and ArrayProxy were implemented as

export let NativeArray = Mixin.create(MutableArray, {
  replace() { replaceInNativeArray(this, ...arguments); }
});

export let ArrayProxy = EmberObject.create(MutableArray, {
  replace() { /* something complex */ }
});

then we would just be adding the function

function replace(array, ...args) {
  if (Array.isArray(array)) {
    replaceInNativeArray(array, ...args);
  } else {
    array.replace(...args);
  }
}

As you can see the duplication of logic is minimal here because the logic is shared in the replaceInNativeArray function. (In fact, that's how the code is arranged on master today: https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/lib/array.js#L18-L24).

This issue is targeted at making these functions public.

[Turbo87]

The RFC does not mention why we have those functions at all. One could imagine that using something like lodash would be another alternative 🤔

[mmun]

I can add an explanation. The answer is that we need them for the same reason that we need get and set, however unlike get and set there's no easy way to intercept array indexing with getters/setters without using Proxy.

It's possible to remove the need for them entirely by making dramatic changes to the Ember programming model (e.g. moving to a more React/Glimmer.js programming model) but that's out of scope for this RFC. I can mention that in the alternatives as well.

[mmun]

Another alternative is to only expose objectAt and replace functions (because all the other methods are based on those). This would mean, though, that something as simple as array.pushObject(item) would need to be written as replace(array, get(array, 'length'), 0, [item])

[Turbo87]

it might be good to mention that/if these new functions are compatible with the proposed pipeline operator

[mmun]

If you grep through https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/mixins/array.js for return this you'll see that several of the existing methods do so I guess the answer is "kind of".

The new functions would have the same inputs/outputs (aside from passing the array in the first argument).

[t-sauer]

Does it make sense to port all available methods? Methods like every, filter or find are available in all supported browsers now, so can't we just get rid of them?

[mmun]

I should list this as (yet another) alternative. Initially I thought the same as you, but then I decided to go with the current proposal to keep things "simpler", e.g. to avoid gotchas like (why can I import mapBy but not map? Both are documented as methods on the Array mixin). I don't have a strong opinion about this.

It's also possible to go to the opposite end of the spectrum and only include objectAt and replace, but see #323 (comment).

[alexlafroscia]

If you think about composing a series of manipulations with the proposed |> operator, having all of the functions (like filter and find) available with the same semantics allows for better consistency

import { filter, mapBy } from '@ember/array';

users
|> filter(u => isAdmin(u))
|> mapBy('name')

I don't think there's a built-in version of filter that takes an array as the first argument

[ryanto]

I think a drawback here is that these will look like "common array functions". Folks might not understand why they need to use the ember version and try to swap it for something like lodash.

Does that mean that this will work...

import { map } from '@ember/array';

map(arr, e => e * 2);

But this would not?

import { map } from 'lodash';

map(arr, e => e * 2);

That might be confusing if we start pushing application developers down this road.

[Turbo87]

see #323 (comment)

"Array" in Ember is essentially an interface, that can have multiple implementations. One implementation is native arrays where lodash will work fine. For other implementations only the Ember functions will work since they are aware of that interface.

[mehulkar]

Does Babel play a role in these array extensions that Ember adds and is that worth addressing here?

[mmun]

@mehulkar Sorry, I'm not sure what you mean.

Maybe related... the RFC should be amended to describe a code mod for add-on authors (or app authors who don't want to use prototype extensions).

[mehulkar]

I think the only "concern" is that if the prototype extensions are removed (and implemented as array functions), then users who are using any of the extensions that are now natively in ES6, would get the native implementation instead. If I am understanding that correctly, then it could be relevant to address in this RFC?

[rwjblue]

I think the only "concern" is that if the prototype extensions are removed (and implemented as array functions), then users who are using any of the extensions that are now natively in ES6, would get the native implementation instead.

Ember always uses the native implementation if present (see implementation here).

[Caltor]

I've just read about this RFC in the Ember Learning Team newsletter. Once implemented should we be starting to write mapBy(array, prop) instead of Array.mapBy(prop) for example in future?

[artursmirnov]

I actually like prototype extensions for the ability to chain operations, which makes code easy to read. Few examples:

With prototype extensions:

items.filterBy('id').mapBy('name').uniq().without('John')

With functions, single line:

without(uniq(mapBy(filterBy(items, 'id'), 'name')), 'John')

Hardly readable

With functions, multiline, for better readability:

const filteredItems = filterBy(items, 'id');
const names = mapBy(filteredItems, 'name');
const uniqNames = uniq(names);
const filteredNames = without(uniqNames, 'John');

Too verbose.

---

Although, I completely understand the Motivation and totally agree. My suggestion would be to create the wrapper functions and instead of implementing them directly, just wrap the passed array into Ember.A, call appropriate method and send back the Ember.A instance. This would allow to keep chainability without prototype extensions and wrapping arrays to Ember.A explicitly.

import { filterBy } from '@ember/array';

filterBy(items, 'id').mapBy('name').uniq().without('John');

Internally filterBy would work like this:

export function filterBy(targetArray, propertyName, propertyValue) {
  return Ember.A(targetArray).filterBy(propertyName, propertyValue);
}

[mmun]

The point of the array functions is to provide an alternative to Ember.A.

I think there's some confusion about what Ember.A does when prototype extensions are disabled. It isn't a wrapper. It assigns every individual Ember array method onto the array instance:

function EmberA(array) {
  if (!PROTOTYPE_EXTENSIONS) {
    array.filterBy = filterBy;
    array.mapBy = mapBy;
    array.uniq = uniq;
    array.without = without;
    // ... many more lines ...
  }

  return array;
}

I don't want to continue encouraging this pattern because it's not performant and it's not idiomatic JavaScript.

[alexlafroscia]

Another issue with prototype extension is that it does not work in FastBoot. Trying to update an application that used it (without realizing) to be FastBoot-friendly was a huge pain. Much better to avoid this pattern entirely.

In terms of chaining, once we have the pipe operator, we'll get a similar pattern for chaining. While I totally agree that this is too verbose

const filteredItems = filterBy(items, 'id');
const names = mapBy(filteredItems, 'name');
const uniqNames = uniq(names);
const filteredNames = without(uniqNames, 'John');

this isn't so bad, IMO

items 
|> filterBy('id')
|> mapBy('name')
|> uniq
|> without('John')

or, as a one-liner

items |> filterBy('id') |> mapBy('name') |> uniq |> without('John')

To me, that reads about the same as the chained-method-calls version, but each function is "pure" and there's no need to mutate the incoming array in any way. Bonus points for the ability to mix some custom function into that chain, which is much harder to do when each operation is a method call.

// Getting friends' names
people |> getFriends |> flatten |> mapBy('name')

// vs.

const friends = flatten(getFriends(people));
const friendsNames = friends.mapBy('name');

[artursmirnov]

@alexlafroscia totally agree, pipe operator would bring the convenience back to the same level. But it's Stage 1 proposal yet, and it may take years to get into our apps (remember decorators). My point is to think about developers ergonomics as well with this change. Because until we have pipe operator we would have to write rather hardly readable or too verbose code. I'm not sure about others but in literally all Ember apps I've been working on, we used those chains a lot.

@mmun thank you for explanation, it totally makes sense. Although falling back to Ember.A was just an idea and more like an illustration of the problem that i'm trying to raise. It could be solved in many different ways. E.g. we may introduce some kind of a piping finction. Something like:

import { pipe, filterBy, mapBy, uniq, without } from '@ember/array';

pipe(items).pipe(filterBy, 'id').pipe(mapBy, 'name').pipe(uniq).pipe(without, 'John').result;

// or even
pipe(items)(filterBy, 'id')(mapBy, 'name')(uniq)(without, 'John').result;

Eventualy pipe operator would naturally replace the pipe() function.

[Serabe]

Is there anything we can do to move this forward?

[thec0keman]

We're going through a fastboot conversion right now and experiencing some of the same pain points. I think this was mostly covered, but wanted to clarify that since Ember prioritizes native functions, chaining is already difficult and surprising. foo.filterBy(a => a.isActive).sortBy('id') won't work, because filterBy falls back to the native filter, and that returns a native array, breaking sortBy.

So while chaining is an important ergonomic to be aware of, it is effectively already broken (and a big troll for developers!) if prototype extensions are disabled.

[rwjblue]

Totally agree @thec0keman! IMHO, we should push folks towards using the native array functions primarily anyways (e.g. instead of filterBy use filter), and use the functions proposed here for mutations.

[pzuraq]

Yeah, so to give a bit more context on the work for autotracking and @tracked, what we have working behind the feature flag today allows us to autotrack any arrays that are being interacted with, without using KVO methods such as objectAt or the wrapper methods that Ember arrays currently implement.

Essentially, it works like native getters when they were added in 3.1. You should be able to read an array in any way you want that is natively supported - including things like destructuring, etc:

class MyComponent extends Component {
  @tracked arr = [1, 3, 2];

  get head() {
    let [head] = this.arr;

    return head;
  }

  get tail() {
    let [_, ...tail] = this.arr;

    return tail;
  }

  get sorted() {
    return this.arr.slice().sort();
  }
}

However, we do still need to intercept updates to arrays, using methods like pushObject, replace, etc. Unfortunately there won't be a way for us to avoid these types of interceptor functions until we can use native JavaScript Proxies.

I've been meaning to update this RFC for a while now, just haven't had the time, but the gist of where it should go, IMO, is only providing:

1. All of the various update functions, such as pushObject and replace.
2. Any non-standard function such as filterBy and mapBy, which should eventually be phased out.
3. objectAt - While not 100% necessary, users will still need to interact with Ember Arrays (and ArrayProxies, Ember Data collections, etc) for some time, and they'll need to use objectAt to do so.

sort is also a bit tricky here, since it sorts the array in place (yay JS), so it maybe should also be included in the update functions. Any other read-only methods that don't mutate the array, like slice, map, and filter don't need to be included at all, because the array class itself should implement them, and they should chain like normal which eliminates that concern.

[NullVoxPopuli]

Do we still need this RFC?

[mmun]

No. The tracked storage primitives and proxies together provide a much more comprehensive solution to this problem in Ember Octane as demonstrated in https://github.com/tracked-tools/tracked-built-ins.