Tresjs / tres

Declarative ThreeJS using Vue Components
https://tresjs.org
MIT License
2.16k stars 102 forks source link

Passing computed or ref values as props triggers a re-renderer #430

Closed alvarosabu closed 6 months ago

alvarosabu commented 11 months ago

Describe the bug

Details on https://discord.com/channels/1047126995424780288/1101057616324603914/1168533469148745768

Reproduction

https://stackblitz.com/edit/stackblitz-starters-c3kpch?file=src%2Fcomponents%2FTheExperience.vue

Steps to reproduce

npm run dev

You will see how the text resets to initial position

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.9.2 - /usr/local/bin/pnpm
  npmPackages:
    @tresjs/cientos: 2.1.2 => 2.1.2 
    @tresjs/core: 2.1.2 => 2.1.2 
    vite: ^4.3.9 => 4.4.11 


### Used Package Manager

npm

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/Tresjs/tres/blob/main/CODE_OF_CONDUCT.md)
- [X] Read the [Contributing Guidelines](https://github.com/Tresjs/tres/blob/main/CONTRIBUTING.md).
- [X] Read the [docs](https://tresjs.org/guide).
- [X] Check that there isn't [already an issue](https://github.com/tresjs/tres/issues) that reports the same bug to avoid creating a duplicate.
- [X] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.
ThimoDEV commented 6 months ago

I did some investigation into the issue and found a potential fix, but i'm not sure is the best solution.

When the dummyComputed triggers a rerender of the component the patchProp function of the custom renderer receives both a prevValue and a nextValue.

When I add an equality check for both and return early the 3D objects position is not reset anymore on trigger

https://github.com/ThimoDEV/tres/tree/local/testing-computed-error-position-reset

Here you can see my experiment, what do you think about this?

andretchen0 commented 6 months ago

This is a fundamental problem of mixing imperative and reactive code. The Tres core could probably be modified to fix this particular instance of the problem. But it's probably best just to avoid the problem altogether, as it's a conceptual one at the root of all the popular reactive frameworks.

Quick fix

In a reactive paradigm, you don't need (and probably shouldn't use) translate. In Vue SFC's ,<template> is declarative, so just declare position to include the translation:

      <Text3D
        :font="fontPath"
        :text="'my 3d text'"
        :size="0.8"
        :position="[num + myTranslateX, num, num]"
      >

Otherwise, to imperatively set the position, e.g., with translate, don't allow <template /> to manage the position.

andretchen0 commented 6 months ago

@alvarosabu

I didn't notice this issue was from 6 months ago. To me, this particular issue can be closed, at least as it stands.

But, along the lines of what @ThimoDEV is rightly suggesting, there's a broader issue of checking if a patchProp actually changes a value. R3F does a similar check.

Maybe that should be opened as a new issue?

alvarosabu commented 6 months ago

I agree @andretchen0 we can close this one and open a feature request to check prev !== next and go for it as long as doesn't affect how the vue renderer API is meant to work.

andretchen0 commented 6 months ago

I agree @andretchen0 we can close this one and open a feature request to check prev !== next and go for it as long as doesn't affect how the vue renderer API is meant to work.

Sounds good. 👍

I suggest that we shoot for incorporating it into a major release. Because changing how the core answers the question "Is this the same value or not?" is bound to break user code somewhere.

Also, thanks @ThimoDEV !