Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DerivedProperty options not flagging excess properties #406

Closed
samreid opened this issue Jul 27, 2022 · 2 comments
Closed

DerivedProperty options not flagging excess properties #406

samreid opened this issue Jul 27, 2022 · 2 comments

Comments

@samreid
Copy link
Member

samreid commented Jul 27, 2022

@marlitas and I observed this code was not failing the type check:

    // map() does not preserve a property of .length required for DerivedProperty
    this.meanProperty = new DerivedProperty( dependencies,
      () => calculateMean( this.getActive3DCups().map( waterCup3D => waterCup3D.waterLevelProperty.value ) ),
      {
        tandem: options.tandem.createTandem( 'meanProperty' ),
        phetioDocumentation: 'The ground truth water level mean.',
        phetioReadOnly: true,
        phetioType: DerivedProperty.DerivedPropertyIO( NumberIO ),
        range: new Range( MeanShareAndBalanceConstants.CUP_RANGE_MIN, 0.8 ),
        hello: 'true',
        thusanthousnatoheusnt: true
      } );

Despite the excess properties at the bottom. Perhaps something about the overloaded DerivedProperty constructor is confusing it.

@samreid
Copy link
Member Author

samreid commented Jul 27, 2022

I found this is not a problem in some cases:

image

samreid added a commit that referenced this issue Jul 28, 2022
@samreid
Copy link
Member Author

samreid commented Aug 2, 2022

I sampled several new DerivedProperty instantiation sites with options, and could not trigger this problem. I think it was due to the inconsistent callback signature, but that has been corrected in MSB by using deriveAny. Closing.

@samreid samreid closed this as completed Aug 2, 2022
zepumph added a commit that referenced this issue Dec 7, 2022
zepumph added a commit to phetsims/sun that referenced this issue Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant