phetsims / phet-core

Core utilities used by all PhET simulations.
MIT License
8 stars 6 forks source link

How any required parameters does `optionize` have? #120

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

Here's an error that I make a few times a day. I've forgotten the second argument (providedOptions) to optionize:

export default class BeamNode extends Rectangle {

  public constructor( providedOptions: BeamNodeOptions ) {

    const options = optionize<BeamNodeOptions, SelfOptions, RectangleOptions>()( {

      // SelfOptions
      beamSize: new Dimension2( 30, 65 )
    } );

This feels buggy, so I'll label it as such -- but I'm not sure. Is there a use-case for calling optionize with a single argument? If not, it would be nice if type-checking could catch this error.

pixelzoom commented 2 years ago

optionize3 apparently has a similar problem. Only 2 arguments in this example, and no TS error:

const options = optionize3<RealLightRaysForegroundNodeOptions, SelfOptions, RealLightRaysNodeOptions>()( {}, {
 stroke: ( GOQueryParameters.debugRays ) ? 'red' : providedOptions.stroke
} );
pixelzoom commented 2 years ago

And shouldn't combineOptions require at least 2 arguments? Otherwise there's no combining happening. This example results in no TS error:

 const arrowNodeOptions = combineOptions<ArrowNodeOptions>( GOConstants.ARROW_NODE_OPTIONS )
zepumph commented 2 years ago

The reason I didn't is this case:

func( providedOptions? MyOptions ){
  const options = combineOptions<MyOptions>( { hi: true }, providedOptions );
}

In this case, since providedOptions could be undefined, it is problematic to not support just a single arg.

pixelzoom commented 2 years ago

Ah, right. I don't see a way around that. So closing.