Problems with BunnyIO

[pixelzoom]

Discovered during https://github.com/phetsims/natural-selection/issues/326 TypeScript conversion...

There are mutliple PhET-iO problems with Bunny.ts. I'll need assistance from @samreid or @zepumph for this issue.

---

• [x] Fields this.father, this.mother, and this.isAlive are currently writeable as part of the Bunny API. They should be readonly, so should be converted to private this._father, this._mother, and this._isAlive, with ES5 getters. This will require modifying stateToArgsForConstructor and applyState. defaultApplyState will no longer work because the core-type field names do not match the stateSchema field names.

---

• [x] this.generation is readonly, so it's inappropriate for applyState to write to it. I should probably reimplement stateToArgsForConstructor so that most of the state (including generation) is restored via constructor, and any remaining state (everything in _private?) is restored via applyState. This feels like a general PhET-iO problem -- when state is restored, TypeScript access modifiers are ignored.

---

• [x] I'm not sure how (or if) this.genotype and this.phenotype are getting restored. Neither of those fields is currently part of BunnyStateObject. Should options.genotypeOptions be included in BunnyStateObject, so it can be used by stateToArgsForConstructor to reconstruct the instance? This feels like a bug, but may just be a misunderstanding by me. I do see this comment in getStateSchema, but I don't understand how this works:

      // genotype and phenotype are stateful and will be serialized automatically.

---

• [x] There is a TypeScript problem with the return type of getStateSchema(). getStateSchema needs to be defined as a method (vs inline as stateSchema: {...} because it references BunnyIO. This is the current implementation:

  private static getStateSchema( BunnyIO: IOType ): CompositeSchema { ... }

  public static readonly BunnyIO = new IOType( 'BunnyIO', {
    valueType: Bunny,
    stateSchema: Bunny.getStateSchema, // TS2322 error here

... and that results in this TS error for the last line:

TS2322: Type '(BunnyIO: IOType<any, any>) => CompositeSchema' is not assignable to type 'StateSchemaOption<any, any> | undefined'.   Type '(BunnyIO: IOType<any, any>) => CompositeSchema' is not assignable to type '(ioType: IOType<any, any>) => StateSchema<any, any>'.     Type 'CompositeSchema' is missing the following properties from type 'StateSchema<any, any>': displayString, validator, compositeSchema, validateStateSchema, and 6 more.

I've also tried this:

  private static getStateSchema( BunnyIO: IOType ): StateSchema<Bunny, BunnyStateObject> {
    return {

      // Even though father and mother are stateful, we need a reference to them.
      father: NullableIO( ReferenceIO( BunnyIO ) ), // TS2322 error here

... but that results in this TS error for the last line:

TS2322: Type '{ father: IOType<any, any>; mother: IOType<any, any>; generation: IOType<number, number>; isAlive: IOType<boolean, boolean>; age: IOType<number, number>; _private: { ...; }; }' is not assignable to type 'StateSchema<Bunny, BunnyStateObject>'.   Object literal may only specify known properties, and 'father' does not exist in type 'StateSchema<Bunny, BunnyStateObject>'.

---

• [x] There is no TypeScript support for the _private part of a StateSchema. See https://github.com/phetsims/tandem/issues/282.

[pixelzoom]

Natural Selection is not currently a publication priority, so unassigning.

[pixelzoom]

• [x] BunnyIO is missing parameterization of IOType. Should be something like:

-  public static readonly BunnyIO = new IOType( 'BunnyIO', {
+  public static readonly BunnyIO = new IOType<Bunny, BunnyStateObject>( 'BunnyIO', {

[samreid]

As part of https://github.com/phetsims/tandem/issues/297, @zepumph and I made it possible for composite state schemas to use underscore keys (as well as continuing to support non-underscored keys), and to access underscored or non-underscored class attributes. There is a TODO that relates to this:

export default class Bunny extends Organism {

  //TODO https://github.com/phetsims/natural-selection/issues/327 father, mother, isAlive, age should be readonly for clients
  public father: Bunny | null;
  public mother: Bunny | null;
  public readonly generation: number;

In this case, if desired, you could rename those like: _father etc and have a public getter (and no public setter). You will likely still need to leave it non-readonly if you want to null it out in dispose. But _father it could be private.

[pixelzoom]

Changes completed in the above commits. Tested in State wrapper. Migraton wrapper indicates that no additional migration rules are needed.

Closing.