TypeScript conversion

[jonathanolson]

Creating an issue to tag commits (for review), and to discuss potential problems in conversion.

[jonathanolson]

@jbphet I converted some tambo bits since Slider depended on the options object for ValueChangeSoundGenerator. Can you review that portion?

[jonathanolson]

InteractionStateProperty looks really tricky here. They use different value sets for different concrete controls, but it's used in e.g. RectangularButton. I've had to type it as IProperty<ButtonInteractionState | RadioButtonInteractionState> in places.

We may need to parameterize the core types e.g. class RectangularButton<InteractionState>, or figure out other solutions.

[samreid]

@chrisklus and I noticed that ReadOnlyProperty was written like this:

export interface ReadOnlyProperty<T> extends Property<T> {
  readonly value: T;

  // Any solution with Omit<Property<T>, 'set' | 'reset'> was failing, as it wasn't assignable to PhetioObject
  set: unknown & any;
  reset: unknown & any;
}

However, this means that this code type checks:

const p = new Property( 1 );
const x: ReadOnlyProperty<number> = p;
x.set( 7 );

Also, we were surprised to see phetsims/center-and-variability@b416354 where an IReadOnlyProperty was converted to Property. It seems it should remain read-only. @jonathanolson can you please advise?

[jbphet]

The 'tambo bits' have been reviewed and look good, though I did make some minor touchups.

[pixelzoom]

@jonathanolson It's been ~30 days since we discussed deleting ReadOnlyProperty, see #726 (comment). If ReadOnlyProperty is not going to be fixed, can we please remove it? More than once, I've accidentally ended up with ReadOnlyProperty when I wanted IReadOnlyProperty. See phetsims/axon#377.

[samreid]

I'll work on removing it momentarily, in phetsims/axon#377

[pixelzoom]

Unfortunately ReadOnlyProperty unintentionally made it into Geometric Optics 1.1. Not too happy about that...

[samreid]

It sounds pretty safe to remove it in a maintenance release commit, if you like.

[samreid]

@jonathanolson can you please report on this status? @zepumph and I are curious whether it's still blocking for the upcoming PhET-iO milestone.

[jonathanolson]

I believe ReadOnlyProperty is now part of the axon hierarchy. @samreid anything left to do here?

[samreid]

To clarify, interface ReadOnlyProperty was removed in phetsims/axon@5a14223. Then in June we added class ReadOnlyProperty in phetsims/axon#382, which has been working much better. So I think that aspect of the issue can be closed.

I'm uncertain about #726 (comment) or the rest of the scope of this issue, up to you.

[pixelzoom]

Remaining TypeScript work is in scenery-phet is tracked in separate issues. So this issue can be closed.