`allowEmpty` class property not respected in v6.3.1 about ember-intl HOT 1 CLOSED

@IgnaceMaes Thanks for reporting a case that I didn't account for in 6.3.0. Namely, an end-developer may override one of the helpers and invert the default value of this.allowEmpty.

In 6.3.0, I removed allowEmpty as a class property, because I found the line options.allowEmpty ?? this.allowEmpty to be confusing (back when ember-intl's helpers inherited a base class). Strangely, {{format-date}} had been the only helper that had set this.allowEmpty to true, so I figured it's unnecessary to have a class property.

I think how you overrode the {{t}} helper and passed allowEmpty: true is okay. It may be a brittle solution, since your helper and how you provide types depend on ember-intl's current implementation.

Because there's another regression from 6.3.0 that I want to fix, I can temporarily revert removing the class property this.allowEmpty so that you can still use the syntax,

import THelper from 'ember-intl/helpers/t';

export default class extends THelper {
  allowEmpty = true;
}

Going forward, what we could try is to make all helpers not throw an error when the value provided is either undefined or null, i.e. we replace the lines,

/* In v6.3.1 */
compute([value]) {
  if (isEmpty(value)) {
    if (options?.allowEmpty ?? this.allowEmpty) {
      return '';
    }

    throw new Error('...');
  }

  // ...
}

with,

/* At a later point */
compute([value]) {
  if (value === undefined || value === null) {
    return '';
  }

  // ...
}

Removing isEmpty() would allow me to do type check more easily, and allow end-developers to skip the step of defining options.allowEmpty. I currently don't see a reason for helpers to throw an error (the intl service could still throw an error, e.g. for missing translations).

from ember-intl.