How would you recommend implementing "enum" props?

[CH-RhyMoore]

This could be a feature request, or it could be a support request.

Feature request

Possibly, an enum prop type. Or some suggestions about a good pattern for them. Or a place to access oldValue when props are being processed.

Use case(s)

My UI components often have options that are chosen by passing props of the string type. But I don't actually want to use any string passed along; I only want to use it if it's one of a few valid and limited option values. From previous ecosystems, I am in the habit of calling these "enum" props.

Some examples:

<ui-icon name="hamburger" align="left" label="Menu"></ui-icon>

In this case, the component has an align property that can be one of top, right, bottom, or left. It defaults to left. As a vanilla WC, I might do something like this:

set align (value) {
  if (value in ALIGN_OPTIONS) {
    // continue and change
  }
  // return and don't change
}

But with Skate, the props are abstracted a little higher for good reasons, which makes me feel limited to the options that are provided and I wasn't satisfied with my first stab:

static get props () {
  return {
    align: Object.assign({}, props.string, {
      default: DEFAULT_ALIGN,
      serialize: function (value) {
        value = props.string.serialize(value);
        if (value in ALIGN_OPTIONS) {
          return value;
        }
        return DEFAULT_ALIGN;
      }
    })
    // ...more code irl
  };
}

In part this is because I think the best behavior isn't to fall back to the default, but rather not to change. If someone initially set right, then accidentally set whee, my opinion is that it's preferable to leave it right-aligned. I'd need some way to get at the old or current value for that, which didn't appear to be currently supported.

<ui-button theme="danger">
  Delete
</ui-button>

In this case, the component has a theme property that can be one of standard, primary, danger, or confirm.

Like the alignment case, there is a default, but passing an unsupported theme can give strange visual results.

[treshugart]

There is a coerce function that you could use instead of serialize. The only argument is the prop value coming in, and the return value is the new value.

In Skate v6, I think I'm dropping most of these in favour of types, but I'm still on the fence. In the proposed v6, this would look like:

// @flow (or use TS)

class MyComponent extends withUpdate() {
  // Build-time checks.
  static props: {
    myEnum: 'val1' | 'val2' | 'val3'
  };
  // Runtime from-attribute handlers.
  static props = {
    myEnum: String
  }
}

The goal here is to simplify runtime logic, but then you don't get this static type checking from HTML which could prove useful. I'm definitely open to input here.

In the proposed scenario, the coercion into an understandable value would happen in render(), if needed, and your component would still have a default. The difference is where the logic happens. This keeps props super simple (and "dumb") and allows you to move logic further down your pipeline where you have access to everything, even previous values (in updating / updated / shouldUpdate).

The added benefit here, is that any translation is transparent to the user. If they set elem.myEnum to something outside of the enum to oiadjfiojasdiofadsiojf, that value doesn't change for them, but you can coerce it to whatever you want internally. You can even log a warning to notify them if you want, without mutating their view of the world.

[trusktr]

In Skate v6, I think I'm dropping most of these in favour of types, but I'm still on the fence.

A types-only API doesn't in most of my cases which don't use Flow or TypeScript. I could still write tests, and/or type checks in coerce or deserialize though, but having it be automatic is nice.

The goal here is to simplify runtime logic, but then you don't get this static type checking from HTML which could prove useful.

That's a very good performance consideration though.

Maybe a "dev" vs "prod" runtime mode can enable/disable the features. It'd be helpful to know things work in dev mode and testing without requiring build steps, and have it lean in production. Maybe there's something we can learn from Vue's "production mode", which is to remove certain checks in the production bundle (or to remove them when there's a capable build system in play).

[CH-RhyMoore]

There is a coerce function that you could use instead of serialize. The only argument is the prop value coming in, and the return value is the new value.

If I have to return something and don't have access to the "old" value, I can't keep from changing the prop when I don't want to, right? (Irrespective of whether that change renders or not.)

The goal here is to simplify runtime logic, but then you don't get this static type checking from HTML which could prove useful. I'm definitely open to input here.

The reasoning around your V6 plans sounds good to me.

But I don't have much context around types, Flow, or TypeScript--I'm an old fashioned front end web developer who has rarely used strong or static types at all and never in JavaScript. So I am doing some confused squinting at the bitwise or operator in that example.

Take this with a grain of salt, as I also don't think in terms of a Reactish component lifecycle; I haven't reached the level of complexity with my Skate work yet to drill it into my head. That may be why I don't find usingshouldUpdate intuitive for this purpose. Enumeration is something I think of in terms of deciding what options my component will have and how those options should work, rather than in terms of skipping renders.

My only concerns would be, since someone can access an element and set its attribute in runtime, and I don't want to write components so opinionated that someone can't do that, it seems to me that there has to be coercion and logic that occur in runtime. Plus, I wouldn't want to be forced to use Flow or TypeScript. I've been doing some component prototypes recently where I don't even build and, god, I miss not having to build. I mean, that's prototypes, not production, and I wouldn't advocate it for production, but still.

(Stencil's use of TypeScript has caused me to repeatedly skip prototyping with it.)

Maybe a "dev" vs "prod" runtime mode can enable/disable the features.

Really like this idea.

[trusktr]

If I have to return something and don't have access to the "old" value, I can't keep from changing the prop when I don't want to, right? (Irrespective of whether that change renders or not.)

Coerce should still return something, because the value gets applied, and same with serialize and deserialize. But you can return the same value (f.e. a reference to the same object, in order to avoid creating a new one, and maybe this isn't very significant with primitives). This would be a lot easier to do if we had access to this of the specific instance; I made an issue about that here: #1396.

[CH-RhyMoore]

I think as I'm reading I'm having a little trouble telling which parts of the comments are addressing which aspects of the issue. There's both:

Short term: best choice for this behavior today in latest stable Skate 5.

Long term: everything else.

Either that or it sounds like there isn't a way to do exactly what I want right now, though there are some ways to get pretty close?

[treshugart]

Yeah, so I'm thinking we need to keep coerce, deserialize and serialize. We should possibly also pass in old / new values to all of them, too. What about name?

[CH-RhyMoore]

@treshugart I can see name being useful. I just got back to looking at this again and was just thinking that about that exact thing.

[trusktr]

The new stuff (v6-trey branch) passes elem into get/set (though I think it should do this in other functions as well), and also you can use this in the prop definition to refer to methods in the prop definition.

I think this mostly has everything you need covered @CH-RhyMoore.

You'll be able to do things like

import {props} from 'skatejs'

const myProps = {
  ...props,

  specificNumber: {
    ...props.number,
    set(elem, propName, val) {
      if (!enumValues.includes(val)) throw new TypeError('invalid value, expected on of: ' + enumValues)
      return val
    }

  },
} 

then use your new prop definition in your class.

[trusktr]

@CH-RhyMoore Well, actually, there's nothing preventing you from doing this currently, as in your case you just need to check an enum, and you don't necessarily need a reference to elem.

So, currently (in v5), you can do

import {props} from 'skatejs'

const myProps = {
  ...props,

  specificNumber: {
    ...props.number,
    coerce(val) {
      val = Number(val)
      if (!enumValues.includes(val)) throw new TypeError('invalid value, expected on of: ' + enumValues)
      return val
    }
  },

} 

and you can throw an error in the case an invalid value is used.

But in v6, it will be more flexible in the sense that you could have per instance or per-class enums, and check those from the element, like this:

const myProps = {
  ...props,

  specificNumbers: {
    ...props.number,
    set(elem, propName, val) {
      if (!elem.enumFor[propName].includes(val)) throw new TypeError('invalid value, expected on of: ' + elem.enumFor[propName])
      return val
    }
  },

} 

and in your class, you might have something like

import Component from 'skatejs'

class MyEl extends Component {
  static props = {
    foo: myProps.specificNumbers,
  }

  // this is custom, used by your customized prop definition
  enumFor = {
    foo: [10, 20, 30] // allow the `foo` prop to be only one of 10, 20, or 30
  }
}

thus it will be easy to create different and dynamic ways of working with props, including reacting differently based on instances, or etc.

[treshugart]

Is there still a need for an enum prop?