Skip to content

fix(core): reuse custom renderer per canvas to stop HMR duplicates#1400

Open
alvarosabu wants to merge 15 commits intomainfrom
fix/hmr-disposal
Open

fix(core): reuse custom renderer per canvas to stop HMR duplicates#1400
alvarosabu wants to merge 15 commits intomainfrom
fix/hmr-disposal

Conversation

@alvarosabu
Copy link
Copy Markdown
Member

Summary

Fixes HMR duplicating scene objects on template edits.

Root cause: Context.vue#mountCustomRenderer was creating a fresh Vue custom renderer on every vite:afterUpdate event. A fresh renderer has no prior vnode tree, so Vue's diff runs against null and mounts everything as new while the old scene objects remain parented to scene — duplicates accumulate every HMR tick.

Fix (R3F-inspired): One Vue custom renderer per canvas, stored in a module-scoped _roots: Map<HTMLCanvasElement, TresRoot>. The map survives HMR because module state persists. On vite:afterUpdate, instead of rebuilding, a reactive hmrTick ref is bumped — the internal component's render function tracks it as a dep and re-runs, re-reading slots.default(). Vue's custom renderer then diffs old vs new and calls nodeOps.remove / patchProp / insert to sync the Three.js scene graph in place.

What changed

  • New packages/core/src/core/roots.ts — module-scoped per-canvas renderer state (getRoot / setRoot / deleteRoot / hasRoot + TresRoot type)
  • Context.vue#mountCustomRenderer is now idempotent — lazily creates one renderer per canvas, reuses on subsequent calls
  • Context.vue#createInternalComponent takes an hmrTick: Ref<number> and reads it reactively inside the render function
  • Context.vue#handleHMRgetRoot(canvas).hmrTick.value++
  • Context.vue#unmountCanvas uses root.renderer.render(null, scene) so Vue walks the vnode tree and disposes via nodeOps.remove, then deleteRoot(canvas)
  • New onBeforeUnmount cleans up the vite:afterUpdate listener (also fixes a pre-existing leak)
  • registerTresDevtools now runs exactly once per mount instead of every HMR tick (pre-existing duplicate-registration bug quietly fixed)

Tests added

packages/core/src/components/TresScene.test/hmr.test.ts (8 tests + 1 documented it.skip):

  • No duplication across simulated HMR tick
  • Slot content diff with a non-reactive mutable (pins hmrTick as the load-bearing trigger)
  • Root entry deleted on unmount
  • Primitive object identity preserved across HMR (<primitive :object="mesh"> keeps the same THREE reference)
  • registerTresDevtools called exactly once per mount
  • Empty default slot + undefined default slot both mount cleanly
  • it.skip for vite:afterUpdate listener cleanup — mocking import.meta.hot doesn't reach Context.vue across vi.resetModules(); cleanup is verified by inspection and documented in the test comment

Plus packages/core/src/core/roots.test.ts with 5 unit tests for the map helpers in isolation.

Full core suite: 528 passed + 3 skipped.

Breaking changes

None on public API. TresCanvasInstance shape is unchanged. TresCanvasInstance.dispose() runtime behavior is preserved — the force parameter on the internal dispose helper was restored after a late review caught that the manual-dispose path had silently lost its WebGL teardown.

Behavioral note worth calling out in release notes: HMR no longer wipes imperative scene state that was previously getting destroyed on every template edit. Most users will experience this as an improvement (OrbitControls target survives, primitive GLTFs don't re-download). Edge-case code that relied on the implicit reset may behave differently.

Ecosystem impact

Package Risk Notes
@tresjs/cientos Low Uses composables, not mount/unmount internals. Worth a smoke test on OrbitControls/CameraControls + loaders.
@tresjs/postprocessing Low-medium <EffectComposer> wraps the WebGL render loop. Worth verifying effect pass HMR.
@tresjs/nuxt Medium (SSR) _roots Map lives at module level. Nuxt's vite-node SSR runtime isolates modules per request, so it should be fine, but warrants an explicit SSR test.
@tresjs/rapier Low Physics composables, no mount/unmount touch.
@tresjs/leches None Standalone.

Test plan

  • Unit tests pass (pnpm --filter @tresjs/core test)
  • Lint + build clean (pnpm --filter @tresjs/core lint / build)
  • Manual HMR verification at /issues/23-hmr-disposal:
    • Baseline scene.children.length counter stable
    • Edit template: add <TresMesh><TresSphereGeometry /></TresMesh> → counter goes up by exactly 1, no duplicates
    • Edit template: remove it → counter back to baseline
    • Rotate with OrbitControls, then edit template → camera angle survives
    • Edit Model.vue script → no duplicates
    • DevTools → inspect scene.children → no orphans
  • Smoke test cientos controls/loaders against the new HMR flow
  • @tresjs/nuxt SSR sanity check

Follow-ups (not this PR)

  • Replace it.skip with an integration test via a src/core/viteHot.ts shim
  • Consider renaming disposedisposeSceneGraph or extracting mountCustomRenderer into createTresRoot.ts
  • Verify Nuxt SSR safety with an explicit test in @tresjs/nuxt

Closes #23

Prevents tests from having to dynamically import roots.ts after
vi.resetModules() — the helper now hands back getRoot/hasRoot/deleteRoot
resolved against the same module graph as Context.vue, so a stale
top-level import cannot silently produce a divergent _roots Map.
Replace reactive ref with a plain mutable so mutation alone does not
trigger re-render. Adds an intermediate assertion after the mutation,
before the tick bump, confirming the scene is unchanged. This proves
the hmrTick.value++ is the actual trigger that causes the internal
component to re-read slots.default(). Previously the test passed even
when handleHMR was gutted — Vue's native reactivity flow was carrying
the slot swap, not the HMR mechanism.
root.renderer is Vue's custom renderer — WebGL teardown is owned by
useRendererManager. The render(null) call here only walks the vnode
tree and disposes CPU-side three.js state via nodeOps.remove.
The previous 'mounts cleanly with no default slot' test actually passed
an empty array via createScene(() => []), which never exercises the
slots.default?.() optional-chain — slots.default is defined, it just
returns []. Split into two tests:

- 'empty default slot' keeps the () => [] case (guards against empty
  scene.children access)
- 'undefined default slot' mounts TresCanvas directly with no children,
  exercising the actual optional chain that Task 2 introduced
The Task 4 dispose simplification correctly avoided double-dispose on
the Vue unmount path (useRendererManager owns renderer teardown there),
but also silently lost the behavior on the manual TresCanvasInstance
.dispose() call — consumers holding a ref and calling .dispose()
expected the previous superset behavior: scene graph walk + renderer
.dispose() + forceContextLoss + renderLists.dispose.

Bring back the `force` parameter. Internal unmountCanvas still calls
dispose(context) (force=false — useRendererManager handles the rest);
the exposed method calls dispose(context, true) to preserve the
consumer-facing contract.
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 11, 2026

Deploy Preview for tresjs-lab ready!

Name Link
🔨 Latest commit 9284eb8
🔍 Latest deploy log https://app.netlify.com/projects/tresjs-lab/deploys/69df79476fb6c60008540b66
😎 Deploy Preview https://deploy-preview-1400--tresjs-lab.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 11, 2026

Deploy Preview for tresjs-docs ready!

Name Link
🔨 Latest commit 9284eb8
🔍 Latest deploy log https://app.netlify.com/projects/tresjs-docs/deploys/69df794778c7e400088006f6
😎 Deploy Preview https://deploy-preview-1400--tresjs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 11, 2026

Deploy Preview for cientos-tresjs ready!

Name Link
🔨 Latest commit 9284eb8
🔍 Latest deploy log https://app.netlify.com/projects/cientos-tresjs/deploys/69df79475009d60008173856
😎 Deploy Preview https://deploy-preview-1400--cientos-tresjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@alvarosabu alvarosabu requested a review from Tinoooo April 11, 2026 21:16
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 11, 2026

Open in StackBlitz

@tresjs/cientos

npm i https://pkg.pr.new/@tresjs/cientos@1400

@tresjs/core

npm i https://pkg.pr.new/@tresjs/core@1400

@tresjs/eslint-config

npm i https://pkg.pr.new/@tresjs/eslint-config@1400

@tresjs/leches

npm i https://pkg.pr.new/@tresjs/leches@1400

@tresjs/nuxt

npm i https://pkg.pr.new/@tresjs/nuxt@1400

@tresjs/post-processing

npm i https://pkg.pr.new/@tresjs/post-processing@1400

commit: 9284eb8

This update includes the import of WebGLRenderer from the three.js library in Context.vue, enhancing the component's capabilities for rendering in a WebGL context.
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 this pull request may close these issues.

HMR disposal

1 participant