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

bug: rerender causes leak for shot cubes #99

Closed
hichemfantar opened this issue Sep 16, 2024 · 5 comments · May be fixed by #102
Closed

bug: rerender causes leak for shot cubes #99

hichemfantar opened this issue Sep 16, 2024 · 5 comments · May be fixed by #102

Comments

@hichemfantar
Copy link

hichemfantar commented Sep 16, 2024

https://github.com/pmndrs/ecctrl/blob/main/example/ShotCube.tsx

Screen.Recording.2024-09-16.at.2.35.28.PM.mp4
  1. open example project
  2. left mouse click to shoot cube
  3. save shotcube.tsx to cause hmr
  4. left mouse click to shoot again
  5. 2 cubes spawn instead of one
  6. save again
  7. shoot again
  8. 3 cubes spawn etc
@hichemfantar hichemfantar changed the title bug: hmr causes buildup fo shot cubes bug: hmr causes leak for shot cubes Sep 16, 2024
@hichemfantar hichemfantar changed the title bug: hmr causes leak for shot cubes bug: rerender causes leak for shot cubes Sep 16, 2024
@ErdongChen-Andrew
Copy link
Member

Refresh the browser after making code changes

@hichemfantar
Copy link
Author

hichemfantar commented Sep 18, 2024

I'm aware of the refresh workaround, however i was thinking of a fix for the leak by probably getting rid of the useState for the cubes and rendering them imperatively. this should solve the issue and even have better performance since the component no longer rerenders needlessly.
I don't see a good reason to use useState for the shot cubes component.
also right now, the cubeRef is being reset for each rendered cube which doesn't seem correct.

@ErdongChen-Andrew
Copy link
Member

That’s just an example, you can improve it however you like in your app. Another improvement could be using instanced cubes

@hichemfantar
Copy link
Author

thanks for the idea, i'm just trying to see what we can do to include good practices as many people will probably use the example as a starting point.

@hichemfantar
Copy link
Author

@ErdongChen-Andrew turns out the issue was an incorrect function reference in the event listener

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

Successfully merging a pull request may close this issue.

2 participants