Alternative ways to specify IO Types

[samreid]

From #188 (comment) and discussion with @zepumph and @pixelzoom.

After #210 ObjectIO constructor will no longer be used to create wrappers.

This opens up the possibility for 2 different ways to specify IO types that may be better in the long run:

(A) Specify the IO type as an object instead of a class. This means instead of:

class BunnyIO extends AnimalIO{
  static documentation = 'documentation';
  static typeName = '...';
  static fromStateObject(){...}
}

It could be declared like so:

const BunnyIO = new IOType({
  documentation: 'documentation',
  typeName: '...'
});

or

class BunnyIO extends IOType{
  toStateObject(){...}
}
return new BunnyIO();

(B) Use the core type as an IO type. For instance, instead of BunnyIO declaring a separate type like so:

class BunnyIO extends ObjectIO {

  // @public @override
  static toStateObject( bunny ) { return bunny.toStateObject(); }

  // @public @override
  static stateToArgsForConstructor( state ) { return Bunny.stateToArgsForConstructor( state ); }

  // @public @override
  static applyState( bunny, stateObject ) { bunny.applyState( stateObject ); }
}

setIOTypeFields( BunnyIO, 'BunnyIO', Bunny );

Instead these fields would be moved to Bunny and it would serve as the IO type.

Constraints and dimensions to consider:

• Subclassing in IO types, also tracking parent IO type automatically
• Parametric IO types
• Convenience and scalability
• Cost to transfer to a new pattern.

[samreid]

A point from @zepumph or @pixelzoom, if we were using a different language we would probably identify IO Type as an interface, to decouple the implementation from the API..

[samreid]

Note: Work for this issue should be done in patches or branches until Natural Selection has safe SHAs.

[samreid]

@zepumph suggested that we could identify APIs for different kids of statefulness. For instance, data types must implement toStateObject/fromStateObject etc. Not sure if we would need an enumeration or some other way to distinguish between these. In Java, it would be a different interface or sub-interface.

[samreid]

Identifying the desired API for an IO Type has been very helpful:

const IOType = {

  // required
  typeName: '',
  supertype: null,
  // validator keys such as isValidValue, valueType or validValues

  // state
  toStateObject: coreObject => {}, // Any stateful PhET-iO Element
  fromStateObject: stateObject => {}, // Data Types only
  stateToArgsForConstructor: stateObject => {}, // Dynamic Elements only
  applyState: ( o, value ) => { }, // Reference Types only

  // optional
  methods: {},
  events: [],
  methodOrder: [],
  parameterTypes: [],
  validator: {}
};

Based on this API, I have sketched out a type class IOType which satisfies this API and has a convenient pattern for declaration sites. I converted several IO Types to use this pattern and it reduces the amount of code in every case significantly. It also automatically validates, which was a sore spot in our previous implementation.

Index: main/tandem/js/types/ObjectIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/types/ObjectIO.js	(revision 9bde4c8e22bd80b8a1ace2a44b3eba7917063bd2)
+++ main/tandem/js/types/ObjectIO.js	(date 1599846443126)
@@ -11,302 +11,12 @@
  * @author Andrew Adare (PhET Interactive Simulations)
  * @author Michael Kauzmann (PhET Interactive Simulations)
  */
-
-import validate from '../../../axon/js/validate.js';
-import ValidatorDef from '../../../axon/js/ValidatorDef.js';
-import merge from '../../../phet-core/js/merge.js';
-import PhetioConstants from '../PhetioConstants.js';
+import IOType from './IOType.js';
 import tandemNamespace from '../tandemNamespace.js';
 
-/**
- * @param {Object} phetioObject
- * @param {string} phetioID
- * @constructor
- * @abstract
- */
-class ObjectIO {
-
-  constructor( phetioObject, phetioID ) {
-    assert && assert( phetioObject, 'phetioObject should be truthy' );
-    assert && assert( phetioID, 'phetioID should be truthy' );
-
-    // @public (read-only)
-    this.phetioObject = phetioObject;
-
-    // @public (read-only)
-    this.phetioID = phetioID;
-
-    // Use the validator defined on the constructor to make sure the phetioObject is valid
-    validate( phetioObject, this.constructor.validator );
-  }
-
-  /**
-   * Return the serialized form of the wrapped PhetioObject. Most often this looks like an object literal that holds the
-   * data about the PhetioObject instance.  This can be overridden by subclasses, or types can use ObjectIO type directly
-   * to use this implementation. This implementation should call `validate()` on the parameter with its `validator` field.
-   * @param {*} o
-   * @returns {*}
-   * @public
-   */
-  static toStateObject( o ) {
-    return o;
-  }
-
-  /**
-   * For data-type serialization. Decodes the object from a state into an instance. This can be overridden by subclasses,
-   * or types can use ObjectIO type directly to use this implementation.
-   * @param {*} stateObject - whatever was returned from the toStateObject method
-   * @returns {*} - the deserialized instance of the same type that the toStateObject was provided as a parameter
-   * @public
-   */
-  static fromStateObject( stateObject ) {
-    return stateObject;
-  }
-
-  /**
-   * Map the stateObject to arguments that are passed to the `create` function in PhetioGroup.js (or other
-   * `PhetioDynamicElementContainer` creation functions). Note that other non-serialized args (not dealt with here) may
-   * be supplied as closure variables. This function only needs to be implemented on IO Types that are
-   * phetioDynamicElement: true, such as PhetioGroup or PhetioCapsule elements.
-   * @param {Object} stateObject - from a corresponding`toStateObject` method
-   * @returns {Array.<*>} - the array of arguments to be passed to creation functions like PhetioGroup.create() or PhetioCapsule.getElement().
-   * @public
-   */
-  static stateToArgsForConstructor( stateObject ) {
-    return [];
-  }
-
-  /**
-   * For reference-type serialization. Applies the stateObject value to the object. When setting PhET-iO state, this
-   * function will be called on an instrumented instance to set the stateObject's value to it.
-   * For more understanding, please read https://github.com/phetsims/phet-io/blob/master/doc/phet-io-instrumentation-guide.md#three-types-of-deserialization
-   * @param {PhetioObject} o
-   * @param {Object} value - result from toStateObject
-   * @public
-   */
-  static applyState( o, value ) { }
-
-  /**
-   * Get the supertype of the passed in IO Type.
-   * @example
-   * ObjectIO.getSupertype( SliderIO )
-   * --> NodeIO
-   * ObjectIO.getSupertype( ObjectIO )
-   * --> null
-   *
-   * @param {function(new:ObjectIO)} ioType
-   * @returns {function(new:ObjectIO)|null} - null if `ioType` is ObjectIO, because that is the root of the IO hierarchy
-   * @public
-   */
-  static getSupertype( ioType ) {
-    assert && assert( ObjectIO.isIOType( ioType ), 'IO Type expected' );
-
-    // getPrototypeOf gets the IO Type's parent type, because the prototype is for the parent.
-    const supertype = Object.getPrototypeOf( ioType );
-    return supertype === Object.getPrototypeOf( window.Object ) ? null : supertype;
-  }
-
-  /**
-   * Returns the type hierarchy for the IO Type, from subtypiest to supertypiest
-   * @param {function(new:ObjectIO)} ioType
-   * @public
-   */
-  static getTypeHierarchy( ioType ) {
-    const array = [];
-    while ( ioType !== null ) {
-      array.push( ioType );
-      ioType = ObjectIO.getSupertype( ioType );
-    }
-    return array;
-  }
-
-  /**
-   * Make sure the IO Type has all the required attributes.
-   * @param {function(new:ObjectIO)} ioType - class to check
-   * @public
-   */
-  static validateIOType( ioType ) {
-
-    const typeName = ioType.typeName;
-    assert && assert( !typeName.includes( '.' ), 'Dots should not appear in type names' );
-
-    // Validate that parametric types look as expected
-    if ( typeName.includes( '<' ) ) {
-      assert && assert( Array.isArray( ioType.parameterTypes ), 'angle bracket notation is only used for parametric IO Types that have parameter IO Types' );
-      ioType.parameterTypes.forEach( parameterType => assert && assert( ObjectIO.isIOType( parameterType ), `parameter type should be an IO Type: ${parameterType}` ) );
-    }
-
-    const splitOnParameters = typeName.split( /[<(]/ )[ 0 ];
-    assert && assert( splitOnParameters.endsWith( PhetioConstants.IO_TYPE_SUFFIX ), 'IO Type name must end with IO' );
-    assert && assert( !ioType.prototype.toStateObject, 'toStateObject should be a static method, not prototype one.' );
-    assert && assert( !ioType.prototype.fromStateObject, 'fromStateObject should be a static method, not prototype one.' );
-    assert && assert( !ioType.prototype.applyState, 'applyState should be a static method, not prototype one.' );
-    assert && assert( !ioType.prototype.stateToArgsForConstructor, 'stateToArgsForConstructor should be a static method, not prototype one.' );
-
-    const supertype = ObjectIO.getSupertype( ioType );
-
-    assert && assert( ioType.hasOwnProperty( 'typeName' ), 'typeName is required' );
-
-    // Prevent inheritance of static attributes, which only pertain to each level of the hierarchy, see https://github.com/phetsims/phet-io/issues/1623
-    // Note that parameterTypes do inherit
-    if ( !ioType.hasOwnProperty( 'events' ) ) { ioType.events = []; }
-    if ( !ioType.hasOwnProperty( 'methods' ) ) { ioType.methods = {}; }
-    if ( !ioType.hasOwnProperty( 'documentation' ) ) { ioType.documentation = {}; }
-    if ( !ioType.hasOwnProperty( 'methodOrder' ) ) { ioType.methodOrder = []; }
-
-    // assert that each public method adheres to the expected schema
-    for ( const method in ioType.methods ) {
-      const methodObject = ioType.methods[ method ];
-      if ( typeof methodObject === 'object' ) {
-        assert && assert( ObjectIO.isIOType( methodObject.returnType ), 'return type must be of type IO: ' + methodObject.returnType );
-
-        assert && assert( Array.isArray( methodObject.parameterTypes ),
-          'parameter types must be an array: ' + methodObject.parameterTypes );
-
-        methodObject.parameterTypes.forEach( parameterType => {
-          assert && assert( ObjectIO.isIOType( parameterType ), 'parameter type must be of type IO: ' + parameterType );
-        } );
-
-        assert && assert( typeof methodObject.implementation === 'function',
-          'implementation must be of type function: ' + methodObject.implementation );
-
-        assert && assert( typeof methodObject.documentation === 'string',
-          'documentation must be of type string: ' + methodObject.documentation );
-
-        assert && methodObject.invocableForReadOnlyElements && assert( typeof methodObject.invocableForReadOnlyElements === 'boolean',
-          'invocableForReadOnlyElements must be of type boolean: ' + methodObject.invocableForReadOnlyElements );
-      }
-    }
-
-    assert && assert( ioType.validator, 'validator must be provided' );
-    assert && assert( ioType.documentation, 'documentation must be provided' );
-    assert && ValidatorDef.validateValidator( ioType.validator );
-
-    ioType.hasOwnProperty( 'methodOrder' ) && ioType.methodOrder.forEach( methodName => {
-      assert && assert( ioType.methods[ methodName ], 'methodName not in public methods: ' + methodName );
-    } );
-
-    if ( ioType.hasOwnProperty( 'api' ) ) {
-      assert && assert( ioType.api instanceof Object, 'Object expected for api' );
-      assert && assert( Object.getPrototypeOf( ioType.api ) === Object.prototype, 'no extra prototype allowed on API object' );
-    }
-
-    // Make sure events are not listed again in the ioType
-    const typeHierarchy = ObjectIO.getTypeHierarchy( supertype );
-    assert && ioType.events && ioType.events.forEach( event => {
-      const has = _.some( typeHierarchy, t => t.events.includes( event ) );
-      assert( !has, 'ioType should not declare event that parent also has: ' + event );
-    } );
-  }
-}
-
-/**
- * Checks if type is an IO Type
- * @param {*} type
- * @public
- * @returns {boolean} - true if inherits from ObjectIO or is ObjectIO
- */
-ObjectIO.isIOType = type => type === ObjectIO || type.prototype instanceof ObjectIO;
-
-/**
- * @typeDef {Object} MethodObject
- * @property {string} documentation
- * @property {function()} implementation - the function to execute when this method is called
- * @property {function(new:ObjectIO)} returnType - the return IO Type of the method
- * @property {Array.<function(new:ObjectIO)>} parameterTypes - the parameter IO Types for the method
- * @property {boolean} [invocableForReadOnlyElements=true] - by default, all methods are invocable for all elements.
- *    However, for some read-only elements, certain methods should not be invocable. In that case, they are marked as
- *    invocableForReadOnlyElements: false.
- */
-
-/**
- * The public methods available for this IO Type. Each method is not just a function, but a collection of metadata
- * about the method to be able to serialize parameters and return types and provide better documentation.
- * @type {Object.<string, MethodObject>}
- */
-ObjectIO.methods = {};
-
-/**
- * The list of events that can be emitted at this level (does not include events from supertypes).
- * @type {string[]}
- * @public
- */
-ObjectIO.events = [];
-
-/**
- * Documentation that appears in PhET-iO Studio, supports HTML markup.
- * @public
- */
-ObjectIO.documentation = 'The root of the wrapper object hierarchy.';
-
-/**
- * The name that this TypeIO will have in the public PhET-iO API. In general, this should only be word characters,
- * ending in "IO". Parameteric types are a special subset of TypeIOs that include their parameters in their typeName.
- * If a TypeIO's parameters are other IO Type(s), then they should be included within angle brackets, like
- * "PropertyIO<Boolean>". Some other types use a more custom format for displaying their parameter types, in this case
- * the parameter section of the type name (immediately following "IO") should begin with an open paren, "(". Thus the
- * schema for a typeName could be defined (using regex) as `[A-Z]\w*IO([(<].*){0,1}`. In most cases, parameterized
- * types should also include a `parameterTypes` field on the TypeIO.
- * @type {string}
- * @public
- */
-ObjectIO.typeName = 'ObjectIO';
-
-/**
- * IO Types can specify the order that methods appear in the documentation by putting their names in this list.
- * This list is only for the methods defined at this level in the type hierarchy.
- *
- * After the methodOrder specified, the methods follow in the order declared in the implementation (which
- * isn't necessarily stable).
- * @type {string[]}
- * @public
- */
-ObjectIO.methodOrder = [];
-
-/**
- * For parametric types, they must indicate the types of the parameters here. 0 if nonparametric.
- * @type {function(new:ObjectIO)[]}
- * @public
- */
-ObjectIO.parameterTypes = [];
-
-/**
- * A validator object to be used to validate the core types that IOTypes wrap.
- * @type {ValidatorDef}
- * @public
- */
-ObjectIO.validator = {
-  isValidValue: value => {
-
-    // It could be a number, string, Object, etc.
-    return value !== undefined;
-  }
-};
-
-ObjectIO.validateIOType( ObjectIO );
-
-/**
- * Fills in the boilerplate for static fields of an IO Type.
- * @param {function} ioType - an IO Type
- * @param ioTypeName - classname of IOType
- * @param coreType - the corresponding Core Type
- * @param {Object} [options]
- */
-ObjectIO.setIOTypeFields = ( ioType, ioTypeName, coreType, options ) => {
-
-  options = merge( {
-    documentation: null // {string} if not provided, default is defined below
-  }, options );
-
-  // Fill in static fields in the IO Type.
-  ioType.typeName = ioTypeName;
-  ioType.documentation = options.documentation ||
-                         `IO Type for ${ioTypeName.substring( 0, ioTypeName.length - PhetioConstants.IO_TYPE_SUFFIX.length )}`;
-  ioType.validator = { valueType: coreType };
-
-  // Verify that the IO Type is valid.
-  ObjectIO.validateIOType( ioType );
-};
+const ObjectIO = new IOType( 'ObjectIO', null, {
+  isValidValue: () => true
+} );
 
 tandemNamespace.register( 'ObjectIO', ObjectIO );
 export default ObjectIO;
\ No newline at end of file
Index: main/tandem/js/types/VoidIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/types/VoidIO.js	(revision 9bde4c8e22bd80b8a1ace2a44b3eba7917063bd2)
+++ main/tandem/js/types/VoidIO.js	(date 1599847315528)
@@ -8,26 +8,9 @@
  */
 
 import tandemNamespace from '../tandemNamespace.js';
+import IOType from './IOType.js';
 import ObjectIO from './ObjectIO.js';
 
-class VoidIO extends ObjectIO {
-
-  constructor( instance, phetioID ) {
-    assert && assert( false, 'should never be called' );
-    super( instance, phetioID );
-  }
-
-  /**
-   * @returns {undefined}
-   * @public
-   */
-  static toStateObject( object ) {
-    return undefined;
-  }
-}
-
-VoidIO.documentation = 'Type for which there is no instance, usually to mark functions without a return value';
-
 /**
  * We sometimes use VoidIO as a workaround to indicate that an argument is passed in the simulation side, but
  * that it shouldn't be leaked to the PhET-iO client.
@@ -35,9 +18,11 @@
  * @override
  * @public
  */
-VoidIO.validator = { isValidValue: () => true };
-VoidIO.typeName = 'VoidIO';
-ObjectIO.validateIOType( VoidIO );
+const VoidIO = new IOType( 'VoidIO', ObjectIO, {
+  documentation: 'Type for which there is no instance, usually to mark functions without a return value',
+  toStateObject: () => undefined,
+  isValidValue: () => true
+} );
 
 tandemNamespace.register( 'VoidIO', VoidIO );
 export default VoidIO;
\ No newline at end of file
Index: main/tandem/js/LinkedElementIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/LinkedElementIO.js	(revision 9bde4c8e22bd80b8a1ace2a44b3eba7917063bd2)
+++ main/tandem/js/LinkedElementIO.js	(date 1599847175854)
@@ -8,36 +8,18 @@
 
 import Tandem from './Tandem.js';
 import tandemNamespace from './tandemNamespace.js';
+import IOType from './types/IOType.js';
 import ObjectIO from './types/ObjectIO.js';
 
-class LinkedElementIO extends ObjectIO {
-
-  /**
-   * @param {LinkedElement} linkedElement
-   * @returns {Object}
-   * @override
-   * @public
-   */
-  static toStateObject( linkedElement ) {
+const LinkedElementIO = new IOType( 'LinkedElementIO', ObjectIO, {
+  documentation: 'A LinkedElement',
+  isValidValue: () => true,
+  toStateObject: linkedElement => {
     assert && Tandem.VALIDATION && assert( linkedElement.element.isPhetioInstrumented(), 'Linked elements must be instrumented' );
     return { elementID: linkedElement.element.tandem.phetioID };
-  }
-
-  /**
-   * @param {Object} stateObject
-   * @returns {Object}
-   * @override
-   * @public
-   */
-  static fromStateObject( stateObject ) {
-    return {};
-  }
-}
-
-LinkedElementIO.documentation = 'A LinkedElement';
-LinkedElementIO.validator = { isValidValue: () => true };
-LinkedElementIO.typeName = 'LinkedElementIO';
-ObjectIO.validateIOType( LinkedElementIO );
+  },
+  fromStateObject: stateObject => {}
+} );
 
 tandemNamespace.register( 'LinkedElementIO', LinkedElementIO );
 export default LinkedElementIO;
\ No newline at end of file
Index: main/phet-io/doc/phet-io-instrumentation-guide.md
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/phet-io/doc/phet-io-instrumentation-guide.md	(revision 46a3986178ad25b133afa31259b7fbe3182ec2b5)
+++ main/phet-io/doc/phet-io-instrumentation-guide.md	(date 1599843809645)
@@ -596,7 +596,7 @@
 Property value. The build-in deserialization method will vary based on the type of deserialization taking place (see 
 below).
 
-Also note that the since work completed in https://github.com/phetsims/phet-io/issues/1398, PhET-iO interframe 
+Also, note that the since work completed in https://github.com/phetsims/phet-io/issues/1398, PhET-iO interframe 
 communications run on structured cloning (via `PostMessage`), and not just JSON strings. This means that a failure to 
 implement a proper `toStateObject` in the IO Type will result in a hard fail when trying to send instances of that type 
 to the wrapper frame. The error will likely look something like this: "Something in message was uncloneable when sent 
Index: main/phet-core/js/EnumerationIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/phet-core/js/EnumerationIO.js	(revision 3c2aaa3b15d2b877997edcab93c35f46aa2f25e8)
+++ main/phet-core/js/EnumerationIO.js	(date 1599847021446)
@@ -6,6 +6,7 @@
  * @author Sam Reid (PhET Interactive Simulations)
  */
 
+import IOType from '../../tandem/js/types/IOType.js';
 import ObjectIO from '../../tandem/js/types/ObjectIO.js';
 import Enumeration from './Enumeration.js';
 import phetCore from './phetCore.js';
@@ -34,53 +35,20 @@
 /**
  * Creates a Enumeration IOType
  * @param {Enumeration} enumeration
- * @returns {function(new:ObjectIO)}
+ * @returns {IOType}
  */
 const create = enumeration => {
-
-  class EnumerationIOImpl extends ObjectIO {
-    constructor( a, b ) {
-      assert && assert( false, 'This constructor is not called, because enumeration values, like primitives, are never wrapped.' );
-      super( a, b );
-    }
-
-    /**
-     * Encodes an Enumeration value to a string.
-     * @param {Object} value from an Enumeration instance
-     * @returns {Object} - a state object
-     * @override
-     * @public
-     */
-    static toStateObject( value ) {
-      return toStateObjectImpl( value );
-    }
-
-    /**
-     * Decodes a string into an Enumeration value.
-     * @param {string} stateObject
-     * @returns {Object}
-     * @override
-     * @public
-     */
-    static fromStateObject( stateObject ) {
-      assert && assert( typeof stateObject === 'string', 'unsupported EnumerationIO value type, expected string' );
-      assert && assert( enumeration.KEYS.indexOf( stateObject ) >= 0, `Unrecognized value: ${stateObject}` );
-      return enumeration[ stateObject ];
-    }
-  }
-
   const toStateObjectImpl = v => v.name;
   const valueNames = enumeration.VALUES.map( toStateObjectImpl );
 
   // Enumeration supports additional documentation, so the values can be described.
   const additionalDocs = enumeration.phetioDocumentation ? ` ${enumeration.phetioDocumentation}` : '';
-
-  EnumerationIOImpl.validator = { valueType: enumeration };
-  EnumerationIOImpl.documentation = `Possible values: ${valueNames}.${additionalDocs}`;
-  EnumerationIOImpl.typeName = `EnumerationIO(${valueNames.join( '|' )})`;
-  ObjectIO.validateIOType( EnumerationIOImpl );
-
-  return EnumerationIOImpl;
+  return new IOType( `EnumerationIO(${valueNames.join( '|' )})`, ObjectIO, {
+    valueType: enumeration,
+    documentation: `Possible values: ${valueNames}.${additionalDocs}`,
+    toStateObject: coreObject => coreObject.name,
+    fromStateObject: stateObject => enumeration[ stateObject ],
+  } );
 };
 
 phetCore.register( 'EnumerationIO', EnumerationIO );
Index: main/tandem/js/types/IOType.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/types/IOType.js	(date 1599846542648)
+++ main/tandem/js/types/IOType.js	(date 1599846542648)
@@ -0,0 +1,54 @@
+// Copyright 2020, University of Colorado Boulder
+
+import validate from '../../../axon/js/validate.js';
+import ValidatorDef from '../../../axon/js/ValidatorDef.js';
+import merge from '../../../phet-core/js/merge.js';
+import PhetioConstants from '../PhetioConstants.js';
+import tandemNamespace from '../tandemNamespace.js';
+
+// constants
+const VALIDATE_OPTIONS_FALSE = { validateValidator: false };
+
+class IOType {
+  constructor( ioTypeName, supertype, config ) {
+    config = merge( {
+
+      // Requires validator, such as isValidValue | valueType | validValues
+
+      // Optional
+      methods: {},
+      events: [],
+      methodOrder: [],
+      parameterTypes: [],
+      documentation: `IO Type for ${ioTypeName.substring( 0, ioTypeName.length - PhetioConstants.IO_TYPE_SUFFIX.length )}`,
+
+      // state
+      toStateObject: coreObject => coreObject, // Any stateful PhET-iO Element
+      fromStateObject: stateObject => stateObject, // Data Types only
+      stateToArgsForConstructor: stateObject => stateObject, // Dynamic Elements only
+      applyState: ( coreObject, value ) => { } // Reference Types only
+    }, config );
+
+    assert && assert( ValidatorDef.containsValidatorKey( config ), 'Validator is required' );
+
+    // @public (read-only)
+    this.typeName = config.typeName;
+    this.methods = config.methods;
+    this.events = config.events;
+    this.methodOrder = config.methodOrder;
+    this.parameterTypes = config.parameterTypes;
+    this.toStateObject = coreObject => {
+      validate( coreObject, config, VALIDATE_OPTIONS_FALSE );
+      return config.toStateObject( coreObject );
+    };
+    this.fromStateObject = config.fromStateObject;
+    this.stateToArgsForConstructor = config.stateToArgsForConstructor;
+    this.applyState = ( coreObject, value ) => {
+      validate( coreObject, config, VALIDATE_OPTIONS_FALSE );
+      config.applyState( coreObject, value );
+    };
+  }
+}
+
+tandemNamespace.register( 'IOType', IOType );
+export default IOType;
\ No newline at end of file

For example, this saves 32 lines of code in EnumerationIO, 15 lines of code in VoidIO and 18 lines of code in LinkedElementIO.

I'm excited about this pattern and think it is the right way to go. However, I am reluctant to start making these changes in our codebase until Natural Selection has SHAs next week.

[samreid]

I added the proposal for IOType.js and ObjectIO2.js (so named so it doesn't interfere with the existing ObjectIO.js). These changes allow BunnyIO to be changed from:

class BunnyIO extends ObjectIO {

  /**
   * Serializes a Bunny instance.
   * @param bunny
   * @returns {Object}
   * @public @override
   */
  static toStateObject( bunny ) {
    assert && assert( bunny instanceof Bunny, 'invalid Bunny' );
    return bunny.toStateObject();
  }

  /**
   * Creates the args that BunnyGroup uses to instantiate a Bunny.
   * @param {Object} state
   * @returns {Object[]}
   * @public
   * @override
   */
  static stateToArgsForConstructor( state ) {
    return Bunny.stateToArgsForConstructor( state );
  }

  /**
   * Restores Bunny state after instantiation.
   * @param {Bunny} bunny
   * @param {Object} stateObject
   * @public
   * @override
   */
  static applyState( bunny, stateObject ) {
    assert && assert( bunny instanceof Bunny, 'invalid Bunny' );
    bunny.applyState( stateObject );
  }
}

ObjectIO.setIOTypeFields( BunnyIO, 'BunnyIO', Bunny );

// @public
Bunny.BunnyIO = BunnyIO;

To:

// @public
Bunny.BunnyIO = new IOType( 'BunnyIO', ObjectIO, {
  valueType: Bunny,
  toStateObject: bunny => bunny.toStateObject(),
  stateToArgsForConstructor: Bunny.stateToArgsForConstructor,
  applyState: ( bunny, stateObject ) => bunny.applyState( stateObject )
} );

Here is a patch that exercises this pattern for other types (basically the uncommitted parts of the above patch)

Index: main/tandem/js/types/VoidIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/types/VoidIO.js	(revision 8c988e53a9c829506f060554db3c13a2aee08294)
+++ main/tandem/js/types/VoidIO.js	(date 1599923914362)
@@ -8,26 +8,9 @@
  */
 
 import tandemNamespace from '../tandemNamespace.js';
+import IOType from './IOType.js';
 import ObjectIO from './ObjectIO.js';
 
-class VoidIO extends ObjectIO {
-
-  constructor( instance, phetioID ) {
-    assert && assert( false, 'should never be called' );
-    super( instance, phetioID );
-  }
-
-  /**
-   * @returns {undefined}
-   * @public
-   */
-  static toStateObject( object ) {
-    return undefined;
-  }
-}
-
-VoidIO.documentation = 'Type for which there is no instance, usually to mark functions without a return value';
-
 /**
  * We sometimes use VoidIO as a workaround to indicate that an argument is passed in the simulation side, but
  * that it shouldn't be leaked to the PhET-iO client.
@@ -35,9 +18,11 @@
  * @override
  * @public
  */
-VoidIO.validator = { isValidValue: () => true };
-VoidIO.typeName = 'VoidIO';
-ObjectIO.validateIOType( VoidIO );
+const VoidIO = new IOType( 'VoidIO', ObjectIO, {
+  documentation: 'Type for which there is no instance, usually to mark functions without a return value',
+  toStateObject: () => undefined,
+  isValidValue: () => true
+} );
 
 tandemNamespace.register( 'VoidIO', VoidIO );
 export default VoidIO;
\ No newline at end of file
Index: main/tandem/js/LinkedElementIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/tandem/js/LinkedElementIO.js	(revision 8c988e53a9c829506f060554db3c13a2aee08294)
+++ main/tandem/js/LinkedElementIO.js	(date 1599923914364)
@@ -8,36 +8,18 @@
 
 import Tandem from './Tandem.js';
 import tandemNamespace from './tandemNamespace.js';
+import IOType from './types/IOType.js';
 import ObjectIO from './types/ObjectIO.js';
 
-class LinkedElementIO extends ObjectIO {
-
-  /**
-   * @param {LinkedElement} linkedElement
-   * @returns {Object}
-   * @override
-   * @public
-   */
-  static toStateObject( linkedElement ) {
+const LinkedElementIO = new IOType( 'LinkedElementIO', ObjectIO, {
+  documentation: 'A LinkedElement',
+  isValidValue: () => true,
+  toStateObject: linkedElement => {
     assert && Tandem.VALIDATION && assert( linkedElement.element.isPhetioInstrumented(), 'Linked elements must be instrumented' );
     return { elementID: linkedElement.element.tandem.phetioID };
-  }
-
-  /**
-   * @param {Object} stateObject
-   * @returns {Object}
-   * @override
-   * @public
-   */
-  static fromStateObject( stateObject ) {
-    return {};
-  }
-}
-
-LinkedElementIO.documentation = 'A LinkedElement';
-LinkedElementIO.validator = { isValidValue: () => true };
-LinkedElementIO.typeName = 'LinkedElementIO';
-ObjectIO.validateIOType( LinkedElementIO );
+  },
+  fromStateObject: stateObject => {}
+} );
 
 tandemNamespace.register( 'LinkedElementIO', LinkedElementIO );
 export default LinkedElementIO;
\ No newline at end of file
Index: main/phet-core/js/EnumerationIO.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- main/phet-core/js/EnumerationIO.js	(revision 3c2aaa3b15d2b877997edcab93c35f46aa2f25e8)
+++ main/phet-core/js/EnumerationIO.js	(date 1599923914336)
@@ -6,6 +6,7 @@
  * @author Sam Reid (PhET Interactive Simulations)
  */
 
+import IOType from '../../tandem/js/types/IOType.js';
 import ObjectIO from '../../tandem/js/types/ObjectIO.js';
 import Enumeration from './Enumeration.js';
 import phetCore from './phetCore.js';
@@ -34,53 +35,20 @@
 /**
  * Creates a Enumeration IOType
  * @param {Enumeration} enumeration
- * @returns {function(new:ObjectIO)}
+ * @returns {IOType}
  */
 const create = enumeration => {
-
-  class EnumerationIOImpl extends ObjectIO {
-    constructor( a, b ) {
-      assert && assert( false, 'This constructor is not called, because enumeration values, like primitives, are never wrapped.' );
-      super( a, b );
-    }
-
-    /**
-     * Encodes an Enumeration value to a string.
-     * @param {Object} value from an Enumeration instance
-     * @returns {Object} - a state object
-     * @override
-     * @public
-     */
-    static toStateObject( value ) {
-      return toStateObjectImpl( value );
-    }
-
-    /**
-     * Decodes a string into an Enumeration value.
-     * @param {string} stateObject
-     * @returns {Object}
-     * @override
-     * @public
-     */
-    static fromStateObject( stateObject ) {
-      assert && assert( typeof stateObject === 'string', 'unsupported EnumerationIO value type, expected string' );
-      assert && assert( enumeration.KEYS.indexOf( stateObject ) >= 0, `Unrecognized value: ${stateObject}` );
-      return enumeration[ stateObject ];
-    }
-  }
-
   const toStateObjectImpl = v => v.name;
   const valueNames = enumeration.VALUES.map( toStateObjectImpl );
 
   // Enumeration supports additional documentation, so the values can be described.
   const additionalDocs = enumeration.phetioDocumentation ? ` ${enumeration.phetioDocumentation}` : '';
-
-  EnumerationIOImpl.validator = { valueType: enumeration };
-  EnumerationIOImpl.documentation = `Possible values: ${valueNames}.${additionalDocs}`;
-  EnumerationIOImpl.typeName = `EnumerationIO(${valueNames.join( '|' )})`;
-  ObjectIO.validateIOType( EnumerationIOImpl );
-
-  return EnumerationIOImpl;
+  return new IOType( `EnumerationIO(${valueNames.join( '|' )})`, ObjectIO, {
+    valueType: enumeration,
+    documentation: `Possible values: ${valueNames}.${additionalDocs}`,
+    toStateObject: coreObject => coreObject.name,
+    fromStateObject: stateObject => enumeration[ stateObject ],
+  } );
 };
 
 phetCore.register( 'EnumerationIO', EnumerationIO );

[samreid]

Above I referenced issues that will be partially or completely addressed by this proposal.

This proposal is at a good point to discuss with @zepumph and @pixelzoom. The main commit is 8c988e5 and a proposed usage is in #211 (comment).

[pixelzoom]

This is looking nicer. I'll probably have some more input when I use it in context. But for now, here are a few thoughts...

• Since supertype can be null, should it be moved into config? I don't feel too strongly about this if ObjectIO is the only case where it will be null.

• Would it be better to have supertype default to ObjectIO? I realize that's a little problematic because ObjectIO2 is defined using new IOType. I don't feel too strongly about this, and maybe it's even better to have ObjectIO explicitly shown as the supertype. Compare:

// @public
Bunny.BunnyIO = new IOType( 'BunnyIO', ObjectIO, {
  valueType: Bunny,
  toStateObject: bunny => bunny.toStateObject(),
  stateToArgsForConstructor: Bunny.stateToArgsForConstructor,
  applyState: ( bunny, stateObject ) => bunny.applyState( stateObject )
} );

vs:

// @public
Bunny.BunnyIO = new IOType( 'BunnyIO', {
  valueType: Bunny,
  toStateObject: bunny => bunny.toStateObject(),
  stateToArgsForConstructor: Bunny.stateToArgsForConstructor,
  applyState: ( bunny, stateObject ) => bunny.applyState( stateObject )
} );

• If super type is null, then toStateObject, fromStateObject, stateToArgsForConstructor, and applyState all become required, since there needs to be default implementations at the root of the IO Type hierarchy. If I understand that correctly, then there should be something that verifies this (assertion? validator? something like ObjectIO.validateIOType?)

[pixelzoom]

I guess another thing I should comment on is whether I think this approach is better than the current approach (see examples in #211 (comment)). It's certainly more concise. But it creates sort of an alternate form of type hierarchy, where you need things like getTypeHierarchy. And that seems more complicated and less straightforward. If we had static constants would this new approach be as attractive? If I had to choose between conciseness and a standard type hierarchy, I'd choose the latter.

[samreid]

All TODOs have been addressed or moved to other issues. The last topic for this issue is how it should be tested. Some options:

1. (lazy) We alert QA to these changes the next time they are dev or RC testing SHAs that contain these changes.
2. (eager) We create a dedicated version and request up-front testing before other SHAs from master are tested.

The lazy approach may be sufficient in this case, but we have discussed moving toward the eager approach for certain things. @zepumph and @ariel-phet what do you recommend?

[zepumph]

I think part of what leans me towards the lazy approach is the difficulty in pin pointing exactly what may have regressed. It may be easier to test eagerly in cases where you can give a runnable to QA and have them go at it. For this issue I would worry about (in no order):

• Type doc changing
• Methods from the wrapper no longer working
• A bug in IOType that causes an API change
• Something that may not have been converted correctly in a specific sim. Hopefully this would get caught on CT, but in the worst case a sim may have a silent state-related bug that.

Nothing above feels like something that QA can tackle efficiently. Likely CT took us 80% there and a code review can take us the rest. What do you think @samreid?

[ariel-phet]

@samreid @zepumph perhaps a bit of both - considering phetsims/qa#519 is high on the testing list currently (EFAC will come first when the next RC is ready, but GAO is actively being tested) - perhaps after some review, GAO will be ready for its next dev test soon and can include these changes with QA alerted. That should help vet the new approach I would think.

[samreid]

Other issues such as
#212
https://github.com/phetsims/phet-io/issues/1709
#213

Should probably be completed before we start testing. They will introduce new complexities that would require further tests.

[samreid]

I opened a dedicated issue about testing and review (which will span the scope of more than this issue). All work complete or tracked elsewhere, closing.