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

Best way to track disposable things in Node #1087

Closed
samreid opened this issue Oct 7, 2020 · 1 comment
Closed

Best way to track disposable things in Node #1087

samreid opened this issue Oct 7, 2020 · 1 comment
Assignees

Comments

@samreid
Copy link
Member

samreid commented Oct 7, 2020

While working on #1047 and #490, @zepumph and I discussed this TODO in Node.js that says:

        // TODO: Should this be set to null in the constructor, https://github.com/phetsims/scenery/issues/490
        const phetioVisibleProperty = new BooleanProperty( this.visible, merge( {

This is later set on the Node for disposal purposes:

        // When created in this manner, the phetioVisibleProperty is "owned" by this Node and we are responsible
        // for its disposal. Assign after calling this.visibleProperty = ... since it disposes this.phetioVisibleProperty
        // if it exists
        this.phetioVisibleProperty = phetioVisibleProperty;

Should we set this.phetioVisibileProperty=null in the constructor? Should we add a Node.disposeEmitter that we append items to?

samreid added a commit that referenced this issue Oct 7, 2020
@zepumph
Copy link
Member

zepumph commented Oct 7, 2020

After discussing with @jonathanolson, we felt like it was best to include a declaration in the Node constructor set to null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants