Computed properties: What am I missing?

Questions General
1

Hi,

The behavior of ember computed properties always suprises me and I would like to have a better understand of it. (I am using Ember 3.4)

Lets say I have two really simple models: a user model and a subscription model. The user has many subscriptions and a subscription always belongs to a user. The state of a subscription represents whether it is active or not.

Models are defined like so:

# models/subscription.js
import DS from 'ember-data';
import { computed } from '@ember/object';

export default DS.Model.extend({
  user: DS.belongsTo('user'),
  state: DS.attr('string'),

  isActive: computed('state', function() {
    return this.get('state') == "active";
  })
})

# models/user.js
export default DS.Model.extend({
  ...
  subscriptions: DS.hasMany('subscription'),
});

Now lets say I want to know if the user has an active subscription: Should be easy, I just have to create a computed property of the user for this:

hasAnActiveSubscription: computed('subscriptions.@each.state', function() {
  this.subscriptions.forEach(subscription => {
    if (subscription.isActive)
      return true;
  })
  return false;
})

Making sure the subscriptions states are loaded using the computed dependence key, then looping through the subscriptions
But this does not work and I can’t seem to get why ?

So I’ve added some debug logs and I inspected my models data with the Ember Inspector:

hasAnActiveSubscription: computed('subscriptions.@each.state', function() {
  this.subscriptions.forEach(subscription => {
    console.log(`Subscription [${subscription.id}] has state: ${subscription.state} - (isActive: ${subscription.isActive})`)
    if (subscription.isActive)
      return true;
  })
  return false;
})

Which gives the following output:

user.js:74 Subscription [1] state: undefined - (isActive: false)
user.js:74 Subscription [1] state: active - (isActive: true)

I can assume to first output is due to a call to the property when the subscriptions have not been fully loaded and the second call is triggered because the subscriptions array have changed and the property needs to be recomputed but I am not quite sure.

What I am sure is that the following solution fixes the problem and I can’t understand what more it is doing under the hood to make it working as expected:

hasAnActiveSubscription: computed('subscriptions.@each.state', function() {
  const activeSubscription = this.subscriptions.findBy('state', 'active')
  return activeSubscription !== undefined
})

I’m pretty sure I am missing something on computed property as I always face problems like that when property are not properly computed. Is there anything obviously wrong with the first code sample I provided ? What makes it wrong ?

2

I think part of the problem is that code isn’t “correct” in the sense that it’s not going to do what you expect. A forEach doesn’t do anything with the return value of each iteration so this CP as written would always loop through all of the subscriptions and then always return false. I think using this.subscriptions.any would make more sense and give you the correct result.

That said your second example is a little more idiomatic:

hasAnActiveSubscription: computed('subscriptions.@each.state', function() {
  const activeSubscription = this.subscriptions.findBy('state', 'active');
  return !!activeSubscription;
})

Additionally, one thing that may help to avoid situations like this in the future is using one or more of the computed macros instead of custom CPs (there are a bunch of options, but something like this):

  activeSubscriptions: filterBy('subscriptions', 'state', 'active'),
  hasAnActiveSubscription: notEmpty('activeSubscriptions')
1 Like
3

Also forgot to mention: this is correct. The CP is calculated once before the subscriptions have finished loading and again once they have resolved. This is necessary when working with computed data based on async data.

1 Like
4

Oh wow, I should have known better… Thanks for correcting such a mistake. I am not used to a return not breaking loops iterations or code blocks but that’s really good to keep in mind for this particular method.

This is one thing that I will be watching very closely from now on as it seems it can make my life easier. I often find myself doing overcomplicated CPs while maybe I could just split it into several macro bits to reach the end result.

Thanks a lot for your help and tips

1 Like