wrapComputed and computedDecoratorWithParams do not work with a getter/setter computed

[lolmaus]

Hi!

This works:

  @computed('startDate')
  get startDateLuxon(): DateTime {
    return DateTime.fromJSDate(this.startDate, { zone: 'UTC' });
  }
  set startDateLuxon(luxonDate: DateTime) {
    this.set('startDate', luxonDate.toJSDate());
  }

But I would like to make this code reusable like this:

@dateToLuxon('startDate')
startDateLuxon!: DateTime;

I'm trying to achieve it with this:

import computed from 'ember-macro-helpers/computed';

// @ts-ignore
import { computedDecoratorWithParams } from '@ember-decorators/utils/computed';

export function dateToLuxonCP(key: any): ComputedProperty<DateTime> {
  return computed(key, {
    get(date: Date) {
      return Date2Luxon(date);
    },
    set(luxonDate: DateTime) {
      this.set(key, luxonDate.toJSDate());
    },
  });
}

// @ts-ignore
export const dateToLuxon = computedDecoratorWithParams((_desc: any, params: any[]) => dateToLuxonCP.apply(this, params));

It doesn't work! this.startDateLuxon returns undefined. I've tried putting a debugger into the getter, and it is never called.

I also tried this and it doesn't work either:

@wrapComputed(dateToLuxonCP('startDate'))
startDateLuxon!: DateTime;

CC @simonihmig.

[rwjblue]

Hmm, this seems more likely to be an issue with ember-macro-helpers doesn't it?

[pzuraq]

@lolmaus first off, @ember-decorators/utils is undocumented private/intimate API. Especially with the computedDecorator helpers, we've made breaking changes in the past without major version bumps, so definitely be aware and use them with caution.

As for the issue, if you wanted to drop ember-macro-helpers, this should work:

import computed from '@ember-decorators/object';

export function dateToLuxon(key: any) {
  return computed(key, {
    get() {
      return Date2Luxon(this[key]);
    },
    set(luxonDate: DateTime) {
      this.set(key, luxonDate.toJSDate());
    },
  });
}

If you want to continue using macro-helpers, then wrapComputed is the public API way to do this, like in your second example. In theory, as long as wrapComputed is passed a valid CP instance it should always work, so something strange is definitely going on if it doesn't.

[lolmaus]

@rwjblue: Hmm, this seems more likely to be an issue with ember-macro-helpers doesn't it?

@pzuraq: In theory, as long as wrapComputed is passed a valid CP instance it should always work

But what is wrong with ember-macro-helpers/computed?

I would like to note the following:

1. ember-macro-helpers/computed has always served me well, I've never had any issues with it.
2. ember-macro-helpers exists to fix issues with Ember API such as syntax inefficiencies and lack of CP composition.
3. Together with ember-awesome-macros, it's my all-time favorite addon. I was expecting ember-decorators to replace it, but since ember-decorators is not gonna support CP composition by design, ember-macro-helpers is there to stay, and it seems to be the way of CP composition in Ember.
4. ember-macro-helpers is quite an infrastructural addon. Numerous apps and several addons depend on it. Even ember-decorators itself used to depend on ember-macro-helpers.

I believe CP composition to be an important feature and humbly ask you to work together with @kellyselden to look into this. :bow: :pray:

PS I'm eager to help too but need some guidance.

[pzuraq]

@lolmaus totally understand the value behind ember-macro-helpers, not trying to dispute that. However, it's a complicated project that uses private Ember APIs to achieve what it's trying to do, and I just haven't had the time to dig in 😩

I can try to give you guidance where possible, but currently my focus is on landing decorators upstream in Ember, and making sure that doesn't completely break ember-macro-helpers (or figuring out the transition path forward)