Typescript Convention: How should we implement enumerations with TypeScript?

[samreid]

From #1049:

How should we implement Enumerations? Should we use string literals or enum? See https://thisthat.dev/literal-union-type-vs-string-enums/. enum would be fine if we have a lint rule for matching key names to values. Otherwise, string literal unions seems good? May still need EnumerationIO for phet-io ones? Or will we want a lightweight EnumerationIO that complements string union? Vanderkam book recommends string literal union since it works via structural typing like the rest of typescript rather than named typing.

Keep in mind the phet-io support needs.

[samreid]

Here's an example of how to map between a string array and a string union type: https://github.com/phetsims/gravity-and-orbits/blob/1652582854fe6bb0ad5f48d748d6f28776474933/js/common/model/GravityAndOrbitsBodies.ts

// DRY string union values and union type: https://stackoverflow.com/questions/44480644/string-union-to-string-array
const GravityAndOrbitsBodies = [ 'planet', 'satellite', 'star', 'moon' ] as const;

gravityAndOrbits.register( 'GravityAndOrbitsBodies', GravityAndOrbitsBodies );

export default GravityAndOrbitsBodies;
export type GravityAndOrbitsBodiesType = ( typeof GravityAndOrbitsBodies )[number];

[samreid]

The reason it seemed we needed the string[] as well as the string union type was that the string[] was being used as a validator in many places. For instance GravityAndOrbitsBodies enumeration was used to validate values for these emitters:

    this.pointAddedEmitter = new Emitter( {
      parameters: [
        { valueType: Vector2 },
        { validValues: GravityAndOrbitsBodies }
      ]
    } );
    this.pointRemovedEmitter = new Emitter( { parameters: [ { validValues: GravityAndOrbitsBodies } ] } );
    this.clearedEmitter = new Emitter( { parameters: [ { validValues: GravityAndOrbitsBodies } ] } );
    this.userModifiedPositionEmitter = new Emitter();
    this.userModifiedVelocityEmitter = new Emitter();

However, with TypeScript, we don't need to validate those values because the type checker does that. Therefore, we could use a pattern more like this:

    this.pointAddedEmitter = new Emitter2<Vector2, GravityAndOrbitsBodies>();
    this.pointRemovedEmitter = new Emitter1<GravityAndOrbitsBodies>();
    this.clearedEmitter = new Emitter1<GravityAndOrbitsBodies>();
    this.userModifiedPositionEmitter = new Emitter0();
    this.userModifiedVelocityEmitter = new Emitter0();

This will allow type checking on the emits and listener callbacks. TypeScript is not yet approved for common code, so investigations along this line would need to be in a branch.

With that pattern, a type union enum would be defined the idiomatic TypeScript way:

type GravityAndOrbitsBodies = 'planet' | 'satellite' | 'star' | 'moon';
export default GravityAndOrbitsBodies;

[samreid]

But I feel we cannot throw away the valid values completely, at least for phet-io usage. When PhET-iO creates radio buttons, it is easier to get the list of valid values for an enum from runtime data, and it cannot (easily?) use type information. So in those cases, we may need to use something like #1106 (comment) at the minimum. Self-unassigning for now.

[samreid]

Our legacy pattern can get TypeScript completion and support by changing from

const TimeSpeed = Enumeration.byKeys( [ 'FAST', 'NORMAL', 'SLOW' ] );

to

const TimeSpeed = new Enumeration( { keys: [ 'FAST', 'NORMAL', 'SLOW' ] } );
TimeSpeed.NORMAL = TimeSpeed.NORMAL; // eslint-disable-line
TimeSpeed.FAST = TimeSpeed.FAST; // eslint-disable-line
TimeSpeed.SLOW = TimeSpeed.SLOW; // eslint-disable-line

Oops, it fails at runtime due to lack of reassignment.

[samreid]

Simply changing:

const TimeSpeed = Enumeration.byKeys( [ 'FAST', 'NORMAL', 'SLOW' ] );

to

const TimeSpeed = new Enumeration( { keys: [ 'FAST', 'NORMAL', 'SLOW' ] } );

passes the type checker, but doesn't offer any kind of type safety. But maybe that's all we need until we have common code support for enums? Or maybe I should just eagerly create a TypeScript-appropriate pattern for enums that has phet-io support.

[samreid]

A proposal for enumerations, based on progressive enhancements:

Level 1. String Literal Union

When you just need to specify one of a few choices, and don't need rich features or PhET-iO, use a string literal union like BodyTypeEnum.ts https://github.com/phetsims/gravity-and-orbits/blob/9322b9a0b9fdcfd4fb2139230a2f338549835b19/js/common/model/BodyTypeEnum.ts

type BodyTypeEnum = 'planet' | 'satellite' | 'star' | 'moon';
export default BodyTypeEnum;

Level 2: Getting values at runtime for a string literal union.

If you also need to get the values at runtime, for validValues or for a PropertyIO, you can use generate the string literal union from the values: https://github.com/phetsims/circuit-construction-kit-common/blob/db384220e00477cc1e9c6a8cb0d3cb5fd08d7624/js/model/CircuitElementViewType.ts#L1-L21

// The values
const CircuitElementViewTypeValues = [ 'lifelike', 'schematic' ] as const;

// The string literal union type
type CircuitElementViewType = ( typeof CircuitElementViewTypeValues )[number];

// Register the values available at runtime.  Note it does not match the filename
circuitConstructionKitCommon.register( 'CircuitElementViewTypeValues', CircuitElementViewTypeValues );

// Export
export { CircuitElementViewType as default, CircuitElementViewTypeValues };

Level 3: Rich enumeration objects.

If you want methods and other rich functionality, you could define it like so (I ported XDirection from natural selection):

// Copyright 2020-2021, University of Colorado Boulder

/**
 * XDirection is the direction that an Organism (bunny, wolf, shrub) is facing along the x axis.
 *
 * @author Chris Malley (PixelZoom, Inc.)
 */

import dotRandom from '../../../dot/js/dotRandom.js';
import circuitConstructionKitCommon from '../circuitConstructionKitCommon.js';

class XDirection {
  sign: number;
  opposite: XDirection | null;
  static LEFT: XDirection;
  static RIGHT: XDirection;

  static getRandom: () => XDirection;
  static KEYS: readonly [ 'LEFT', 'RIGHT' ];
  static phetioDocumentation: string;
  static VALUES: readonly XDirection[];

  constructor( sign: number ) {
    this.sign = sign;
    this.opposite = null;
  }
}

XDirection.LEFT = new XDirection( -1 );
XDirection.RIGHT = new XDirection( +1 );

XDirection.KEYS = [ 'LEFT', 'RIGHT' ] as const;
XDirection.VALUES = [ XDirection.LEFT, XDirection.RIGHT ] as const;

XDirection.getRandom = () => dotRandom.nextBoolean() ? XDirection.RIGHT : XDirection.LEFT;
XDirection.phetioDocumentation = 'hello';

circuitConstructionKitCommon.register( 'XDirection', XDirection );
export default XDirection;

And the IO type that makes it work in studio would be something like:

// Copyright 2018-2021, University of Colorado Boulder

/**
 * IO Type for phet-core Enumeration that supports serializing and deserializing values. Cannot be moved to the core
 * type since Enumeration must be defined before ValidatorDef can be defined.
 *
 * @author Sam Reid (PhET Interactive Simulations)
 */

import IOType from '../../../tandem/js/types/IOType.js';
import StateSchema from '../../../tandem/js/types/StateSchema.js';

// {Map.<enumeration:Enumeration, IOType>} - Cache each parameterized RichEnumuerationIO so that it is only created once.
const cache = new Map();

type RichEnumeration<U> = {
  KEYS: readonly string[]
  VALUES: readonly U[]
  phetioDocumentation: string
};

/**
 * This caching implementation should be kept in sync with the other parametric IO Type caching implementations.
 * @param {Object} enumeration
 * @returns {IOType}
 */
const RichEnumuerationIO = <U, T extends RichEnumeration<U>>( enumeration: T ) => {

  if ( !cache.has( enumeration ) ) {
    const values = enumeration.VALUES;
    const getKey = ( value: T ) => enumeration.KEYS.find( key =>

      // @ts-ignore
      enumeration[ key ] === value );

    // Enumeration supports additional documentation, so the values can be described.
    const additionalDocs = enumeration.phetioDocumentation ? ` ${enumeration.phetioDocumentation}` : '';

    cache.set( enumeration, new IOType( `RichEnumuerationIO(${enumeration.KEYS.join( '|' )})`, {
      validValues: values,
      documentation: `Possible values: ${enumeration.KEYS.join( ', ' )}.${additionalDocs}`,
      toStateObject: getKey,
      fromStateObject: ( stateObject: string ) => {
        assert && assert( typeof stateObject === 'string', 'unsupported RichEnumuerationIO value type, expected string' );
        assert && assert( enumeration.KEYS.indexOf( stateObject ) >= 0, `Unrecognized value: ${stateObject}` );

        // @ts-ignore
        return enumeration[ stateObject ];
      },
      stateSchema: StateSchema.asValue( `${enumeration.KEYS.join( '|' )}`, {
        isValidValue: ( v: string ) => enumeration.KEYS.includes( v )
      } )
    } ) );
  }

  return cache.get( enumeration );
};

export default RichEnumuerationIO;

The call site looks like:

private readonly directionProperty: Property<XDirection>;
 // ...
    this.directionProperty = new Property( XDirection.LEFT, {
      validValues: XDirection.VALUES,
      tandem: tandem.createTandem( 'directionProperty' ),
      phetioType: Property.PropertyIO( RichEnumerationIO<XDirection, typeof XDirection>( XDirection ) )
    } );

Remaining things to do:

• How can we support legacy Enumeration values? Maybe for now convert Enumeration.byKeys to new Enumeration.
• Unify the above approaches?
• Discuss if it's OK to use PropertyIO(StringIO) for Level 2?
• Improve the IO type for the Level 3 case. And the Level 3 case repeats itself a lot.

[samreid]

Here are 2 ways we can provide typing for legacy enumuerations:

Using strings (allows passing 'CATS')

( () => {
  type AnimalType = 'CATS' | 'DOGS';
  type AnimalTypeEnumerationType = {
    CATS: AnimalType
    DOGS: AnimalType
  };
  const AnimalTypeEnumeration = Enumeration.byKeys( [ 'CATS', 'DOGS' ] ) as unknown as AnimalTypeEnumerationType;

  const property = new Property<AnimalType>( AnimalTypeEnumeration.CATS );
  property.set( AnimalTypeEnumeration.MONKEY );
  property.set( 'lion' );
  property.set( 'CATS' );
  property.set( AnimalTypeEnumeration.DOGS );
  console.log( property.value );
} )();

Using enums (requires AnimalTypeEnumeration.CATS)

( () => {

  enum PetKind { //eslint-disable-line
    CATS,//eslint-disable-line
    DOGS//eslint-disable-line
  }

  const AnimalType = Enumeration.byKeys( [ 'CATS', 'DOGS' ] ) as unknown as ( typeof PetKind );

  const property = new Property<PetKind>( AnimalType.CATS );
  property.set( AnimalType.MONKEY );
  property.set( 'lion' );
  property.set( 'CATS' );
  property.set( AnimalType.DOGS );
  console.log( property.value );
} )();

We could pick one of these strategies, or one like it, and use it to set up d.ts files next to legacy Enumeration files. I'm not sure how to say EnumerationProperty<T> extends Property<T> -- that is the same type parameter in both cases. Maybe that part would require TS in common code? Or maybe Property<MyEnum> could be used as a workaround until then?

Here's a picture showing the underlined type errors:

[samreid]

I reviewed these proposals with the dev team today, and we decided @zepumph and I should discuss further and bring a proposal to next Thursday's meeting.

Questions:

• Can we automatically pluck validValues or the phetioType instead of having to repeat those?
• If we convert Orientation.VERTICAL to 'vertical', can you search for all occurrences? Yes. See below:
• Can/Should we get a better matchup with existing legacy code?

In this code, searching for usages of 'satellite' finds the first case but not the second case.

type BodyTypeEnum = 'planet' | 'satellite' | 'star' | 'moon';

const createBody = ( x: BodyTypeEnum ) => {

};

createBody( 'satellite' );

const printName = ( x: string ) => {
  console.log( x );
};

printName( 'satellite' );

[samreid]

I built a type to automatically set validValues and phetioType:

class EProperty<T> extends Property<T> {
  constructor( values: readonly T[], value: T, options?: any ) {
    options = options || {};
    options.validValues = values;
    options.phetioType = Property.PropertyIO( StringIO );
    super( value, options );
  }
}

const p2 = new EProperty<CircuitElementViewType>( CircuitElementViewTypeValues, 'lifelike' );
p2.link( ( e: CircuitElementViewType ) => {
  console.log( e );
} );

p2.value = 'lifelike';
p2.value = 'schematic';

[samreid]

I moved the examples to wilder and embellished upon them a bit.

[samreid]

@zepumph and I met about this today. We searched for a strategy that would create a type from an array of strings like new MyEnumeration(['CAT','DOG']); but could not get something like that working. We also discussed the searchability of string literal union values like in #1106 (comment), and confirmed it was working well in all the cases we tried except for Property.js (though when we converted it to Property.ts it was OK).

For this pattern:

    class MammalType {
      static PUPPY = new MammalType();
      static KITTY = new MammalType();
    }

@zepumph was inclined to make it extend an enumeration type like:

    class MammalType extends Enumeration{
      static PUPPY = new MammalType();
      static KITTY = new MammalType();
    }

But it is unclear to me what problem that would solve. We also discussed that one of the nice parts of the wilder options demo is that it shows a more extreme complexity. We could add another example to the enumeration patterns that shows features like:

• Making one enum value the opposite of another, like we do in some rich enums
• Adding constructor arguments to the enumeration constructor.

We also ran out of time and I wasn't sure we were on the same page about next steps or current status. We will discuss again next week. I feel my perspective here is incomplete, so would be good for @zepumph to chime in about important parts I missed.

I also identified Bounds2 as an example of a type that has a static value of its own type:

Bounds2.EVERYTHING = new Bounds2(...);

I think the reason we are having trouble matching TypeScript to our legacy pattern is that our legacy Enumeration class is value-oriented, and we want to promote it to type-oriented. That's why it seems we need a new pattern.

[pixelzoom]

In case you haven't run across it... https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums:

In modern TypeScript, you may not need an enum when an object with as const could suffice:
...
The biggest argument in favour of this format over TypeScript’s enum is that it keeps your codebase aligned with the state of JavaScript, and when/if enums are added to JavaScript then you can move to the additional syntax.

[zepumph]

I added a couple more changes. Please review.

[pixelzoom]

The substance of the documentation looks good, but the execution needed some work -- see commits above.

The number of ways that "TypeScript" and "JavaScript" are spelled seems to be a chronic problem. It's (maybe) OK to be fast & loose in comments to yourself. But in public documents, it's distracting, and conveys an absence of attention-to-detail. My 2 cents... I'll keep changing them where I see them.

@zepumph feel free to close.

[zepumph]

Makes sense to me! I'll too be on the lookout. Thank you.