Error when using computed values with ember-decorators-polyfill installed or Ember 3.10.0

[jderr-mx]

Environment

• Ember Version: 3.6 with ember-decorators-polyfill or current 3.10.0-beta.2
• Ember CLI Version: 3.6
• Ember CP Validations Version: 3.5.5 and 4.0.0-beta.7

Steps to Reproduce

I tested this with my work app at 3.8 and used the dummy app at 3.6 and installed ember-decorators-polyfill

Having validations that use a form of computed for their value. models/user-detail.js - dob validator

after: computed(function() {
  return moment()
    .subtract(120, 'years')
    .format('M/D/YYYY');
}).volatile(),

Results in the error: Error: Assertion Failed: EmberObject.create no longer supports defining computed properties. Define computed properties using extend() or reopen() before calling create().

I tracked it in the debugger to -private/options.js in the constructor for the Options class

constructor({ model, attribute, options = {} }) {
  const optionKeys = keys(options);
  const createParams = { [OPTION_KEYS]: optionKeys, model, attribute };

  // If any of the options is a CP, we need to create a custom class for it
  if (optionKeys.some(key => isDescriptor(options[key]))) {
    return OptionsObject.extend(options).create(createParams);
  }

  return OptionsObject.create(createParams, options);
}

I chatted with Chris Garrett in e-decorators on the Ember discord. He suggested that the computed properties were not being detected and that you would always need to extend. He said the same error would apply with Ember 3.10 and I have been able to reproduce it with the dummy app and ember-source upgraded to 3.10.0-beta.2

I attempted update the code to always return OptionsObject.extend(options).create(createParams);

Which got rid of the error but then some of the tests were not passing.

Please let me know if there is more detail that I can provide.

[offirgolan]

It seems like the issue here has something to do with the isDescriptor utility method. Do you happen to know why that might be?

[jderr-mx]

So I did a little debugging and checked ember 3.6 with ember-decorators-polyfill:

• typeof "function"
• isDescriptor is true
• constructor dec - constructor.name "DecoratorDescriptor"
• https://github.com/pzuraq/ember-decorators-polyfill/blob/v1.0.3/vendor/ember-decorators-polyfill/index.js#L209

ember 3.10.0-beta.2

• typeof "function"
• isDescriptor is undefined
• constructor COMPUTED_DECORATOR - constructor.name "ComputedDecoratorImpl"
• https://github.com/emberjs/ember.js/blob/master/packages/%40ember/-internals/metal/lib/decorator.ts#L139

[Alonski]

@jderr-mx Did you ever find a solution to this? @offirgolan Any ideas how to fix?

[jderr-mx]

I started digging into the tests that are failing and it looks like the dependent keys for the model in theoptions object are not being recognized when the CPs are created.

I think it is caused because rather than an instance of a ComputedProperty object the computed decorator is a function. The result is there is no _dependentKeys array available when extractOptionsDependentKeys runs.

function extractOptionsDependentKeys(options) {
  if (options && typeof options === 'object') {
    return Object.keys(options).reduce((arr, key) => {
      let option = options[key];

      if (isDescriptor(option)) {
        return arr.concat(option._dependentKeys || []);
      }

      return arr;
    }, []);
  }

  return [];
}

For reference, I have been debugging this test: Integration | Validations | Factory - General: disabled validations - cp with dependent key

[jderr-mx]

I was able to work around this issue by creating a dependentKeys object on the validation. This got all the cp related tests passing in the PR @Alonski submitted.

    let Validations = buildValidations(
      {
        firstName: validator('inline', { validate: Validators.presence }),
        lastName: validator('inline', {
          validate: Validators.presence,
          disabled: not('model.validateLastName')
        })
      },
      {
        dependentKeys: ['model.validateLastName']
      }
    );

Inheritance integration tests in Factory - General are failing in Ember < 3.9 The Parent fields are not getting inherited by the child.

[alejandrodevs]

Getting the same error in Ember 3.10.0-beta.1. Any ideas how to fix it?

[Alonski]

@alejandrodevs Can you try to point to this branch? #636 See if it works until we have a more permanent fix

[alejandrodevs]

@Alonski I tried it and the error is still present.

Ember: 3.10.0-beta.1. Ember CP Validation: Your branch

[jderr-mx]

For 3.10 I had to update addon/-private/options.js to make the constructor always extend the object.

export default class Options {
  constructor({ model, attribute, options = {} }) {
    const optionKeys = keys(options);
    const createParams = { [OPTION_KEYS]: optionKeys, model, attribute };
    
    return OptionsObject.extend(options).create(createParams);
  }
}

[alejandrodevs]

@jderr-mx That fixes my issue. Could that have side effects in other versions?

[jderr-mx]

@ alejandrodevs I'll start by running the tests with ember-try and see what comes up.

[offirgolan]

For 3.10 I had to update addon/-private/options.js to make the constructor always extend the object.

This is very expensive to do and I would prefer to fix the underlying isDescriptor utility method instead if at all possible.

[jderr-mx]

For 3.10 I had to update addon/-private/options.js to make the constructor always extend the object.

This is very expensive to do and I would prefer to fix the underlying isDescriptor utility method instead if at all possible.

That utility method relies on the property .isDescriptor which is no longer present in 3.10+ What would be a safe property to test?

[Alonski]

@offirgolan from what I understood from @pzuraq we should always extend the object. With decorators you don't know if a function is a descriptor or not.

[pzuraq]

@offirgolan what they're saying is correct, the goal in the near future is to eliminate all of the the intimate/private APIs for computed properties. There are a few that have to remain around for the time being, but where there are alternatives we're trying to push people towards them. I think extending is a reasonable alternative, and it will be the only alternative in the future with native classes, so I would recommend moving to something like this now, or not using computed properties.

[jderr-mx]

Any additional thoughts on how to update this addon?

Decorators have shipped in 3.10 which will cause a couple of issues if you are using a computed property in a validator, to disable it for example.

• EmberObject.create no longer supports defining computed properties. issue
• model.<someAttr> will need to be explicitly added to dependentKeys

[alejandrodevs]

For 3.10 I had to update addon/-private/options.js to make the constructor always extend the object.

export default class Options {
  constructor({ model, attribute, options = {} }) {
    const optionKeys = keys(options);
    const createParams = { [OPTION_KEYS]: optionKeys, model, attribute };
    
    return OptionsObject.extend(options).create(createParams);
  }
}

What about doing this to fix the first issue? EmberObject.create no longer supports defining computed properties.

[alejandrodevs]

When do we need to do this model.<someAttr> will need to be explicitly added to dependentKeys?

[jderr-mx]

When do we need to do this model.<someAttr> will need to be explicitly added to dependentKeys?

See this example: https://github.com/offirgolan/ember-cp-validations/issues/635#issuecomment-485315979

The disabled attribute of lastName is set using a computed property

[alejandrodevs]

@jderr-mx Oh I see! at least we can get ride of that issue using dependentKeys in this moment. I think the first point should be fixed as it has a higher priority.

Any of us has worked on fixing that issue?

[Alonski]

@alejandrodevs I have been trying to fix this. Running into a few issues. Check out this PR: #636

[alejandrodevs]

This fix by @GavinJoyce is working well. https://github.com/offirgolan/ember-cp-validations/pull/636#issuecomment-494330352

[GavinJoyce]

Is the support for decorators in this addon widely used? If not, perhaps we could consider dropping them?

[Alonski]

@GavinJoyce I managed to fix the issue with Decorators in your branch I think. I will try to find time in the next few days to make a PR.

What are we going to do about versions older than 3.7 though?

[GavinJoyce]

@Alonski nice one, perhaps you could push up what you have?

What are we going to do about versions older than 3.7 though?

My changes work with earlier versions of Ember too.

[GavinJoyce]

Is the support for decorators in this addon widely used? If not, perhaps we could consider dropping them?

Or perhaps we could move the decorator support to an addon which wraps this one?

[lolmaus]

Or perhaps we could move the decorator support to an addon which wraps this one?

This looks the most reasonable. Keep decorators available without making the main addon unstable.

[GavinJoyce]

https://github.com/offirgolan/ember-cp-validations/pull/641 is 🍏. It drops support for decorators and supports all versions of ember from 2.16 -> 3.10

[lolmaus]

I'm running into the same issue when using the ember-intl's t CP macro inside an ember-awesome-macros CP macro.

import { and, or } from 'ember-awesome-macros';
import { t } from 'ember-intl';

This works:

  @t('assumptionGroup.options.partTime')
  partTimeName!: string;

  @t('assumptionGroup.options.pattern')
  patternName!: string;

  @or(
    and('isPartTime', 'partTimeName'),
    and('isPattern',  'patternName'),
                      'name'
  )
  nameEffective!: string;

This doesn't (but it reads much better!):

  @or(
    and('isPartTime', t('assumptionGroup.options.partTime')),
    and('isPattern',  t('assumptionGroup.options.pattern')),
                      'name'
  )
  nameEffective!: string;

The latter produces: Error: Assertion Failed: EmberObject.create no longer supports defining computed properties. Define computed properties using extend() or reopen() before calling create().

Should I start a separate issue? In which repo?