Closed pixelzoom closed 1 year ago
Tagging for https://github.com/phetsims/phet-io/issues/1914 and self-unassigning.
This would come in handy for https://github.com/phetsims/center-and-variability/issues/204
I'm enjoying this patch very much:
The 2 holdouts on this pattern are:
../../../axon/js/DerivedProperty.ts:129:13 - error TS2345: Argument of type '{ tandem: Tandem; }' is not assignable to parameter of type 'LinkedElementOptions'.
Object literal may only specify known properties, and 'tandem' does not exist in type 'LinkedElementOptions'.
129 tandem: dependenciesTandem.createTandemFromPhetioID( dependency.tandem.phetioID )
~~~~~~
../../../joist/js/preferences/PreferencesModel.ts:451:62 - error TS2345: Argument of type '{ tandem: Tandem; }' is not assignable to parameter of type 'LinkedElementOptions'.
Object literal may only specify known properties, and 'tandem' does not exist in type 'LinkedElementOptions'.
451 this.addLinkedElement( modelPropertyObject.property, { tandem: tandem.createTandem( tandemName ) } );
~~~~~~
Found 2 errors in 2 files.
Errors Files
1 ../../../axon/js/DerivedProperty.ts:129
1 ../../../joist/js/preferences/PreferencesModel.ts:451
I think we should figure out a way to make this work.
Heads up to @pixelzoom @jbphet @zepumph that I'm hoping to commit numerous cross-repo changes for this issue soon.
@samreid, I'm confused by the current state of this issue. In a conversation with you, I thought we discussed that these two "holdouts" were reasonable usages, and that it is acceptable to allow tandem
as an option for edge cases. What do you recommend is next for this change?
@samreid and I cleared it up. Thanks!
One more TODO here.
The current API for PhetioObject
addLinkedElement
is:Two suggested improvements:
addLinkedElement
look like this:That's boilerplate, much of it unnecessary. Change the implementation of
addLinkedElement
so that the above can be written as:... and
addLinkedElement
defaults the tandem tothis.tandem.createTandem( someElement.tandem.name )
.This would also address the problem that
options?
is optional, butaddLinkedElement
fails if you don't provideoptions.tandem
.options.tandemName
instead ofoptions.tandem
. If you're callingthis.addLinkedElement
, then the tandem should bethis.tandem.createTandem( options.tandemName )
. With the current API, callingthis.addLinkedElement
does not guarantee that the linked element will appear under this.tandem in the Studio tree. For example, there's nothing to prevent this: