phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

Memory leak in TextPushButton #833

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

Discovered during https://github.com/phetsims/reactants-products-and-leftovers/issues/78.

TextPushButton is a subclass of RectangularPushButton that creates its own content:

    options.content = new Text( text, combineOptions<TextOptions>( {
      font: options.font,
      fill: options.textFill,
      maxWidth: options.maxTextWidth,
      tandem: options.tandem.createTandem( 'text' )
    }, options.textNodeOptions ) );

There are 2 memory leaks associated with options.content:

(1) It registers a tandem, added by @zepumph (2) It may be like to a TReadOnlyProperty<string>, added by @jonathanolson

So TextPushButton needs to override dispose and handle this.

pixelzoom commented 1 year ago

The tandem leak was introduced first, so assigning to @zepumph to review. Close if OK.

zepumph commented 1 year ago

Yes very good. Thanks for the catch.