search

phetsims / axon

Axon provides powerful and concise models for interactive simulations, based on observable Properties and related patterns.
MIT License
10 stars 8 forks source link

Multilink thinks second parameter can be null when it cannot #380

Closed zepumph closed 2 years ago

zepumph commented 2 years ago

I found a bug with the typing of Property.multilink. I'm unsure how to fix it, but it has a really easy reproducible case. By having the third Property which is of type string|null, the second arg in the callback thinks it could be null, but it can't. The assignment in the callback fails with this error:

TS2322: Type 'string | null' is not assignable to type 'string'.   Type 'null' is not assignable to type 'string'.

    const x = new Property<string>( 'hi' );
    const y = new Property<string>( 'hi' );
    const z = new Property<string | null>( 'hi' );
    Property.multilink( [ x, y, z ], ( xValue, yValue, zValue ) => {
      const foo: string = yValue;
    } );
zepumph commented 2 years ago

@jonathanolson, can you assist?

zepumph commented 2 years ago

I was able to easily workaround this by providing the parameters to multilink manuall (<[string, string, string|null]>), but likely we want this fixed generally since my workaround is a code smell when it could infer them.

samreid commented 2 years ago

This appears fixed in the latest implementation of multilink. I added that test code and saw it seemed to be type checking OK.

image

@zepumph can this issue be closed?