Data: Computed `record.myHasMany.[]` doesn't fire if nothing accesses `record.myHasMany`

[Keeo]

It looks like its no longer possible to listen on hasMany array length (model.foos.[]) and synchronously get the size of it.

// works in ember-data 2.10 but not later
fooLength: Ember.computed('model.foos.[]', function() {
  return this.get('model').hasMany('foos').ids().length;
}),

I created twiddle - you can see that if you change ember-data to older version it works.

[nadavshatz]

We've been seeing this as well.

But mostly for deleting a hasMany object. we are on 2.12

[stefanpenner]

Did a quick debug, didn't quite get to the bottom of it. But I believe am seeing the hasMany be notified of a change...

Needs more investigation.

[nadavshatz]

Don't know if it helps but I actually had to do this to resolve it:

  destroyRecord() {
    return this.get('parent').then(parent => {
      this.deleteRecord();

      return this.save().then(() => {
        user.notifyPropertyChange('hasChildren');
      });
    });
  }

I also noticed the issue very clearly when reaching an empty state.

Let me know if I can assist further in deugging this.

[stefanpenner]

Let me know if I can assist further in deugging this.

Porting the above twiddle to a failing test would be very helpful, and also give us a starting point for fixing/adding additional test coverage.

[nadavshatz]

@stefanpenner no problem - I'll give it a go.

Where would be the best place to look at examples of testing computed properties based on relationships?
I see stuff in the integration folder for one-to-many relationships but nothing around computed properties.

Thanks!

[stefanpenner]

I would write up something that mimics the twiddle in this test: https://github.com/emberjs/data/blob/master/tests/integration/relationships/has-many-test.js

More or less what you have in the twiddle should do (but without factory-guy and stuff)

[nadavshatz]

@stefanpenner please take a look at #4976
More than happy for any comment/guidance.

it is currently failing.

[nandorstanko]

Not sure is it the same issue, but this doesn't work as well, after adding new comments and changing their attributes. The observer fires when old (loaded) comments are changed, but doesn't when changing the newly added ones. It was working with version 2.12.2.
```javascript
commentsObserver: Ember.observer('[email protected]', function() {
//save changed comments
})

[Keeo]

Any update on this issue? It has been present since v2.11 and is still broken in current 2.16/master.

[VladislavCohen]

Hi all, can anybody help to resolve this issue, please?

[mwpastore]

@Keeo Typo in your original example. Should be fooLength: Ember.computed('model.foos.[]', function() { (to match the hasMany).

@nadavshatz Typo in your workaround. Should be parent.notifyPropertyChange('hasChildren'); (to match the function argument).

Just to throw a wacky ol' wrench in the works, I can get the failing test to pass by changing this:

  hasComments: computed('comments.[]', function() {
    return this.hasMany('comments').ids().length > 0;
  })

To this:

  hasComments: computed('comments.[]', function() {
    return this.get('comments.length') > 0;
  })

Which doesn't really make sense, but there it is.

[nadavshatz]

@mwpastore won't you're solution depend on actually loading the relation vs doing it without going to the server?

[mwpastore]

@nadavshatz Yeah, possibly. I'm not sure if it will just read the length off the PromiseManyArray without triggering a fetch or not.

[nadavshatz]

@mwpastore at least for my point - that was the goal, to be sure to do so without loading the relationship.

[mwpastore]

@nadavshatz Absolutely. I just thought it was worth pointing out that it works. It doesn't make any sense (to me, at least) how changing the CP function body would affect property change notification. In other words, why does the CP fire once with this.hasMany('comments').ids().length and twice with this.get('comments.length')? Clearly something else is going on here.

[burritoIand]

+1 I just spent ~4 hours debugging why my computed properties that spy on a hasMany relationship were not updating when the related data changed. I then realized that I had just upgraded from 2.12.1 to 2.17.1beta last week so I downgraded back to 2.12.1 and everything works again. This is a major bug for us and prevents us from upgrading past 2.12.1. I tried 2.13.1 and it does not work which confirms that something after 2.12.x broke computed properties on related fields.

To give more background:

I have some computed properties that look like such:

  mail: hasMany('letter', { async: true }),

  outgoingMail: computed('mail.[]', {
    get() {
      return this.get('mail')
        .rejectBy('acceptedAt')
        .filterBy('isOutgoing', true);
    }
  }),

I have a template that looks like this:

{{#if user.outgoingMail}}...we have mail{{/if}}

And a route-action that does this:

  _addLetter(letter, now=new Date()) {
    return this.get('store').createRecord('letter', {
      requestedAt: now,
      sender: letter.get('sender'),
      user: this.currentUser.get('content'),
    }).save()
  },

In 2.17.1beta or anything past 2.12.x calling _addLetter does NOT update user.outgoingMail

[Keeo]

@erichonkanen There has been talk about this and other similar bugs on slack lately.

[burritoIand]

@Keeo ok thanks I'm going to look more into it tomorrow.. this combined with soft delete issue is causing some serious pains!

[runspired]

@mwpastore I think your solution actually pin-points the bug

[burritoIand]

I went back and tried to reproduce this issue (computed properties on model relationships) and was able to get it working by adding a second field-level key to my computed property:

incomingFoos: computed(
      'foos.[]',
      'foos.@each.{createdAt}', {
    get() {
      return this.get('foos')
        .rejectBy('createdAt')
        .filterBy('isBar', false);
    }
  }),

If I only have the CP on foos.[], when I create a new foo, the computed property does not update. However with above code it does...

[runspired]

I dug in last night and found a fix but it is extremely dirty. The underlying problem is the mixing of two entirely different views of the world. I will have a PR up in an hour with this fix and a lengthy explanation, but I am unsure we will choose to accept the PR given there is a clear work around.

[runspired]

@erichonkanen your issue is separate from this and has to do with a bug in your code. You are treating async relationships as if they are sync within your computed properties.

[st-h]

@runspired thanks a lot for your work and update. I had noticed that what @erichonkanen mentioned actually improves the situation a bit when using async relationships correctly. However, I also noticed that observing dedicated properties does not work as expected when a new record is created and persisted. I noticed things like the observer triggering when the record is created but not when the value is changed or persisted. After reading your PR I was wondering if you think this is a related issue or something totally different?

[burritoIand]

@runspired what's the correct way to resolve the code I had pasted w/computed property dependent on async array? will using e.g. {{#if (await user.outgoingMail)}} work?

[Subaku]

Came across this issue and wanted to show how I've gotten around it in case it helps others in the meantime. I've got a couple of models that both have computed properties pointing at their hasMany relationship. In my UI I have forms where I create new instances for these hasMany relationships so in my components, right after I save successfully, I manually remove the newly saved object and then push it back in again which triggers my computed properties.

// Model for a Report
export default DS.Model.extend({
    schedules: DS.hasMany('report-schedule'),
    managedSchedule: computed('[email protected]', function() {
        // Returns the schedule I want.
    }),
});

After saving a new ReportSchedule I would execute:
schedule.get('report').get('schedules').removeObject(schedule).pushObject(schedule);

I did this for both my affected models and so far so good.

Thank you to everyone who is working on this :)

UPDATE

So an update on my previous solution. After further refactoring my previous solution stopped working for just one of my models. In particular it's the one I posed a code example of. My other model which has a computed (the one that still works with the above solution) declares its property a little differently which I neglected to mention.

// Model for ReportSchedule
export default DS.Model.extend({
    destinations: DS.hasMany('report-destinations'),    
    managedDestinations: computed('destinations.[]', function() {
        // Returns the destination I want.
    }),
});

So I needed to find a new solution for my schedules model.

let report = this.get('store').peekRecord('report', schedule.get('report.id'));
report.notifyPropertyChange('managedSchedule');

IMPORTANT! This only worked for me if I fetched the report from the store and NOT from the belongsTo relation off the schedule you saw me do in my previous solution. Weird...

Lastly since it might be helpful/important for others my versions of things:

Ember: 3.0.0
Ember Data: 3.0.0

Cheers

[runspired]

This should no longer be an issue in ember-data 3.5+ as lazy relationships were removed