[Bug] 2.3.2 broke `observes` modifier

[esbanarango]

🐞 Bug description

With the latest version of ember-concurrency the observes modifier stopped working.

import Model, { attr } from '@ember-data/model';
import { inject as service } from '@ember/service';
import { task } from 'ember-concurrency';

export default class Parking extends Model {
  @service store;

  @attr('array', { defaultValue: () => [] }) carIds;

  // This tasks should perform everytime `cardIds` changes.
  @task({
    observes: 'carIds.[]',
    drop: true,
  })
  *loadCarsTask() {
    // ...
  }
}

import Component from '@glimmer/component';
import { action, set } from '@ember/object';

export default class ParkingBox extends Component {
  @action
  onCarChange(cars) {
    const { parking } = this.args;
    const cardIds = cars.map(({ id }) => parseInt(id));

    set(parking, 'cardIds', cardIds);
  }
}

😕 Actual Behavior

loadCarsTask isn't performing when carIds changes.

🤔 Expected Behavior

loadCarsTask should perform everytime cardIds changes.

🌍 Environment

• ember-concurrency 2.3.2
• ember-source 4.6.0
• ember-data 4.2.0

[machty]

@esbanarango did you also bump ember-source when you upgraded ember-concurrency? Asking because observers are going to be less and less supported until they're finally removed (and i'm kinda surprised they're still working in 4.6.0)

[esbanarango]

@machty We didn't bump those two together. observes functionality was working with:

ember-concurrency 2.2.1 ember-source 4.6.0 ember-data 4.2.0

[machty]

@esbanarango did you get rid of any references to ember-concurrency-decorators or any other relevant details?

[esbanarango]

Yeah, no usage of ember-concurrency-decorators at all.

[robclancy]

Seems like on: 'init' isn't working either (always felt dirty using it anyway, just being lazy). Downgrading to find version where it does work until we update everything to the new syntax.

Edit: 2.2.1 works

[buschtoens]

I can confirm the { on: 'init' } bug as well. Fortunately we were only using this in a single service, which we fixed by calling .perform() in the init() hook.

[machty]

I think it makes sense to mark this as wontfix, not out of laziness, but because Ember is heading in a different direction.

The technical reason these broke is that the new API is assigning a property at object initialization time, whereas the previous approaches manipulate the prototype (a requirement for event-based / observer stuff to work). There are probably ways to fix these APIs, but observers are already deprecated and with on init you can just put the perform in the constructor.

Happy to hear feedback on this but otherwise we're probably not gonna fix this.

[esbanarango]

@machty, Agreed on moving forward with Ember's direction. But this breaking change should probably be added in a major release.

[machty]

@esbanarango ohh... I apologize; to clarify, we shouldn't support observes in the new API, but I agree we should fix whatever broke about the old API. I'll take a closer look shortly.

[machty]

Fixed in v2.3.4

[esbanarango]

@machty this is still failing if we use the new "syntax":

Fails

// Won't perform when `cardIds` changes
loadCarsTask = task(
  {
    observes: 'carIds.[]',
    drop: true,
  },
  async () => {

  }
)

Works fine

  @task({
    observes: 'carIds.[]',
    drop: true,
  })
  *loadCarsTask() {
    // ...
  }

This's maybe fine for now, but worth mentioning.

[machty]

@esbanarango Yes, I think it is probably not a good idea for us to try and fix this for the new API as it would add further complexity to the EC codebase to accommodate a deprecated pattern.

[AmilKey]

@machty it is possible to add information on the new syntax about these events on: "init", on: "didInserElement", observes: "", because I spent a lot of time trying to figure out why it stopped working after upgrading to a new version