Text/RichText constructor and setTextProperty take different Property types.

[pixelzoom]

The textProperty parameter to Text/RichText constructors differs from setTextProperty:

  public constructor( text: string | number | TReadOnlyProperty<string>, options?: TextOptions ) {
...
  public setTextProperty( newTarget: TProperty<string> | null ): this { 

This makes it impossible to provide a DerivedProperty to Text/RichText via setTextProperty. Shouldn't setTextProperty also take a TReadOnlyProperty<string>?

I have an immediate need for this in molecule-polarity RealMoleculeViewer.ts:

    const moleculeText = new RichText( '', {
      font: FONT,
      maxWidth: 200
    } );

    moleculeProperty.link( molecule => {
      moleculeText.textProperty = new DerivedProperty(
        [ molecule.fullNameProperty, moleculePolarityStrings.pattern.symbolNameStringProperty ],
        ( fullName, patternString ) => StringUtils.fillIn( patternString, {
          symbol: molecule.symbol,
          name: fullName
        } ) )
    } );

The assignment to moleculeText.textProperty results in this tsc error:

TS2322: Type 'DerivedProperty<string, string, string, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown>' is not assignable to type 'TProperty'.   Type 'DerivedProperty<string, string, string, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown>' is not assignable to type '{ set(value: string): void; value: string; }'.     Property 'set' is protected in type 'DerivedProperty<string, string, string, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown, unknown>' but public in type '{ set(value: string): void; value: string; }'.

I can workaround this in RealMoleculeViewer.ts by creating a new RichText node instead of calling setTextProperty. But that will be trouble when I need to instrument for PhET-iO.

[jonathanolson]

I was told to follow the pattern with Node.visibleProperty (I didn't invent this pattern!)

moleculeText.mutate( { textProperty: ... } ) was proposed as the way of handling this.

Perhaps we can improve this pattern @zepumph? Thoughts @pixelzoom?

[jonathanolson]

Node's visibleProperty is TReadOnlyProperty in the options, TProperty in the setVisibleProperty() definition.

[pixelzoom]

Thoughts @pixelzoom?

It's really weird, and problematic in practice, that the constructor takes a (readonly) TReadOnlyProperty<string>, while setTextProperty takes a (writeable) TProperty | null ). What is it about setTextProperty` that require a writeable Property?

[jonathanolson]

What is it about setTextProperty` that require a writeable Property?

My understanding was that we want to be able to access things that TProperty provides from getters, e.g.

text.textProperty.value = would be illegal if it's a TReadOnlyProperty.

That doesn't sound like a good argument to me, and perhaps we can change them all to TReadOnlyProperty?

[zepumph]

How about this @jonathanolson? If you agree with it than I can apply it to all Node Properties over in phetsims/axon#391

[zepumph]

Just a ping here. Then I can proceed with phetsims/axon#391.

[zepumph]

I discussed with @jonathanolson and we like the above commit. @pixelzoom you can now set a DerivedProperty onto Text.stringProperty. Feel free to close!

[pixelzoom]

👍🏻 closing