mapBy: Dependent keys containing @each only work one level deep

[lolmaus]

Here's a reproduction: https://ember-twiddle.com/645211e42dc9b18d3f06f390d1fa8475?openFiles=controllers.application.js%2C

  array: [
    {foo: {bar: 'baz'}},
    {foo: {bar: 'quux'}},
    {foo: {bar: 'zomg'}},
    {foo: {bar: 'lol'}},
  ],
  
  mapped: mapBy('array', raw('foo.bar')),

I do understand that this will not recompute when a bar changes. But that's still a legit use case. We have tons of static data we want to transform with mapBy('array', raw('foo.bar')) and instead we have to use old-school CPs like this:

mapped: map("array.[]", function (item) {	
   return item.get("foo.bar");	
}),old

Splitting into two CPs should resolve the issue:

  mapped2foos: mapBy('array',       raw('foo')),
  mapped2:     mapBy('mapped2foos', raw('bar')),

but that's still too wordy for a use case where data does not change. It's also slower (two loops instead of one).

[kellyselden]

I believe that's a limitation set by Ember and can't be fixed here. Correct me if I'm wrong.

[lolmaus]

The problem is with ember-awesome-macros: it generates a CP dependency path like foo.@each.bar.baz. It should drop levels of depth past the first one.

[kellyselden]

Hmm. I think that would be confusing as to why we aren't watching the correct property. Maybe the error from Ember.computed is the best scenario in this case.

[lolmaus]

You already admitted that Ember does not support watching more than one level of depth, so why do it?

[kellyselden]

I'm saying if Ember.computed('array.@each.foo.bar', ...) and mapBy('array', raw('foo.bar')) give you the same error, than that is good enough. No need to make this project more complex just to hide an Ember issue from you. Maybe we could give a better error message, but an error is probably the right thing to do.

[lolmaus]

The matter is that an array.mapBy("foo.bar") expression is legit and does not produce any warning. It's a valid use case that I believe may be pretty common.

Say, I have an array with static, non-changing objects. I want to map those objects by a nested property. I tell the addon to do that and it spits a warning. I write the same CP by hand and it works as expected without any warnings.

The fact that mapBy('array', raw('foo.bar')) transforms into an equivalent of Ember.computed('array.@each.foo.bar', ...) is a bug in the addon. Specifically, normalizeArrayKey produces a faulty 'array.@each.foo.bar' dependency key that Ember correctly warns about. Such key should never exist in the first place.

It is not an Ember issue. By saying "No need to make this project more complex" you're achieving the opposite: users of your addon have to be aware that it contains a pitfall and think of ways of working around it.

I strongly believe that the issue should be fixed in ember-awesome-macros. The warning about non-observable nested properties should be a caveat in the readme section of the mapBy CP macro.

PS I'm a huge fan of yours. ember-macro-helpers and ember-awesome-macros are by far my most favorite addons. I use them whereever possible and encourage my teammates to do the same. It makes code so much more concise and thus more maintainable! Having to revert to conventional computed properties is a pain.

[mriska]

I agree with @kellyselden here. Silently dropping the second-level bar from the dependency key would work in your specific case since your data is static. But it would lead to issues that are difficult to debug for others that expect the dependency keys to be honoured. I second the idea of keeping the error and letting the developer know it would not work the way he might be expecting.

[kellyselden]

I tell the addon to do that and it spits a warning. I write the same CP by hand and it works as expected without any warnings.

You can't.

mapped: Ember.computed('array.@each.foo.bar', function() {
  return this.get('array').map(array => array.get('foo.bar'));
})

is invalid. Anything shorter like mapped: map("array.[]" doesn't watch the correct property, so you can't make the argument that the same CP works in Ember.

[lolmaus]

This is the CP that I need:

mapped: Ember.computed('array.@each.foo', function() {
  return this.get('array').mapBy('foo.bar');
})

It works correctly, and when array content is static (which is very common for stuff like Elastic search results), not observing bar is a non-issue.

[mriska]

If that is what you need couldn't you just use the builtin computeds?

mapped: Ember.computed.mapBy('array', 'foo.bar')

It seems to me like the demands that you have is not exactly what this addon so excellently provides? It is nice if a solution fits everyones needs, but that is in some cases impossible to accommodate. If it can't work for everyone I would aim for least surprises to the developer which I believe is the current functionality.

[lolmaus]

If that is what you need couldn't you just use the builtin computeds?

No, that also produces a warning. Something that could be worked around in an addon.

[mriska]

You are right of course, that would trigger the same error message.

It could be worked around in a addon, but then it would confuse everybody that does not have the exact same needs as you, since it didn't work the way the developer expects.

[lolmaus]

@mriska What exactly will "not work"?

[mriska]

If i write mapped: mapBy('array', raw('foo.bar')) and it does not give me an error I expect it to recompute if the bar property changes.

[lolmaus]

I've just realized this:

array.map('array.@each.foo', item => get(item, "foo.bar"))