Cannot read properties of undefined (reading 'dispose') - GridHelper

[whitespacecode]

Describe the bug

When i try to show/hide the GridHelper i get a TypeError. The code still works.
I tried it on an older tresjs version and no typeError happend.

<TresGridHelper v-if="gridHelper.visible" :args="[10, 100, 0xff0000]" />

Not sure if that is also the case for other components.

Reproduction

https://stackblitz.com/edit/tresjs-basic-hfgtuj

Steps to reproduce

Use the reproduction, click on the button 'Toggle Grid' and check the console logs.

System Info

System:
    OS: macOS 14.3.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 90.73 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.0 - ~/.nvm/versions/node/v18.18.0/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v18.18.0/bin/yarn
    npm: 10.6.0 - ~/Code/Modulab/node_modules/.bin/npm
  Browsers:
    Chrome: 125.0.6422.142
  npmPackages:
    @tresjs/cientos: ^3.9 => 3.9.0
    @tresjs/core: ^4.0.2 => 4.0.2
    vite: ^5.2.10 => 5.2.10

Used Package Manager

npm

Code of Conduct

• I agree to follow this project's Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• The provided reproduction is a minimal reproducible example of the bug.

[garrlker]

Looks like we're calling .dispose() twice when this is removed from the scene. Easy fix, though it's interesting how this bug got in

[alvarosabu]

@andretchen0 is this solved by the disposal refactor?

[andretchen0]

@alvarosabu

is this solved by the disposal refactor?

Most likely. I will make a test from the reproduction and add it to the nodeOps tests.

[andretchen0]

Following up, this is indeed a problem with Tres nodeOps (Vue's RendererOptions) remove.

This test against main currently fails:

  it('disposes a GridHelper', () => {
    const gridHelper = nodeOps.createElement('TresGridHelper', undefined, undefined, {})
    expect(() => nodeOps.remove(gridHelper)).not.toThrow()
  })

[andretchen0]

@garrlker

I see you're assigned here.

Fwiw, the issue is fixed in a soon-to-be submitted PR dealing with primitive/disposal.

When I submit, I'll mark that PR as closing this issue, if that's ok.

[andretchen0]

Just for future reference, it is interesting:

Looks like we're calling .dispose() twice when this is removed from the scene. Easy fix, though it's interesting how this bug got in

The bug stems from these 2 competing disposals:

• THREE creates the GridHelper along with its material and geometry. When its dispose method is called, the THREE code tries to this.material.dispose(). It assumes that this.material exists – which is a perfectly valid assumption, since THREE put it there.
• Tres disposes from the bottom up. Children are disposed and detached before parents.

By the time Tres calls dispose on the GridHelper, Tres has already disposed and removed its material and geometry.

That makes THREE's perfectly valid assumption wrong, unfortunately. THREE tries to do this.material.dispose(), which throws because Tres had removed this.materal.