Discussion about required interface

[SaladFork]

Spoke with @jnhuynh about this via Slack, but repeating here for discussion. There are several directions that the addon can take in terms of how it exposes its API.

Current implementation: Enforcer.required returning a mixin

const ButtonComponent = Ember.Component.extend(
  Enforcer.required('height'),
  Enforcer.required('width', { type: 'number' }),
  Enforcer.required('model', { type: 'object' }),
  Enforcer.required('enabled', { type: 'bool' }),
  Enforcer.required('color', { message: 'Component requires hex string attribute' }),
  Enforcer.required('onClick', {
    type: 'function',
    message: 'Component requires onClick callback attribute'
  }),
  {
    enabled: true
  }
);

Notes:

• Are there any existing examples of functions returning mixins?

  Proposal: single mixin w/ class property:

const ButtonComponent = Ember.Component.extend(EnforcerMixin, {
  requiredAttrs: [
    'height',
    'width:number',
    'model:object',
    'enabled:bool',
    'color',
    'onClick'
  ],

  enabled: true
});

Notes:

• How would you pass a message option?
• Maybe this can be similar to the queryParams property where it can be either an array or a hash?
• The : separation of this array approach seems to be going out of style

  Proposal: required property value similar to Ember.inject/DS.attr

const ButtonComponent = Ember.Component.extend({
  height: Enforcer.required(),
  width: Enforcer.required('number'),
  model: Enforcer.required('object'),
  enabled: Enforcer.required('bool', { defaultValue: true }),
  color: Enforcer.required({ message: 'Component requires hex string attribute' }),
  onClick: Enforcer.required('function', { message: 'Component requires onClick callback attribute' })
});

Notes:

• Type-less color option is inconsistent in that it gets options hash as first argument, but is consistent with DS.attr
• Difficult to make the default value a computed property that depends on something else

Are there any benefits / drawbacks to any of these? A reason to choose one over another?

[SaladFork]

Proposal: Computed property method similar to volatile and readOnly

const ButtonComponent = Ember.Component.extend({
  height: Ember.computed(...).required(),
  width: Ember.computed(...).required('number'),
  /// ...
});

[elwayman02]

I like the second proposal of a single mixin taking an array of properties. To handle messages and options, you could just do this:

const ButtonComponent = Ember.Component.extend(EnforcerMixin, {
  enforcedAttrs: [
    'height',
    'width:number',
    'model:object',
    { name: 'enabled', type: 'bool' },
    { name: 'color', message: 'A color is required, you idiot.' },
    'onClick'
  ],

  enabled: true
});

You'll note I changed the proposed name from requiredAttrs to enforcedAttrs...I felt it lowered the chance of collision with property names people might want to use without actually making the API any less intelligible.

My main concern with this addon is that failing to provide a required attribute crashes an app entirely. I'm not sure if there is a better way, but I don't know that I like white page syndrome being the result of this addon's features.

On a related note, what about allowing component developers to choose whether to throw an error or a warning? They may have attributes that are "required" but won't cause severe problems if omitted; perhaps the component fails gracefully but won't truly work without the missing attribute. Or maybe this addon could actually have 3 options:

1. Fail via Ember.Assert
2. Fail via Ember.Logger.error (which wouldn't kill the app outright, I think)
3. Warning via Ember.Logger.warn

Thoughts @SaladFork @jnhuynh?

[lukemelia]

I like the idea of configurable failure modes. Perhaps this could/should be a app-level configuration, so that you could make it crash in test, but warn in dev.

[elwayman02]

:+1: for app-level base config, override-able on specific properties as needed