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

fix: don't recursively dispose primitives #3087

Merged
merged 1 commit into from
Nov 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/fiber/src/core/renderer.ts
Original file line number Diff line number Diff line change
@@ -212,7 +212,7 @@ function createRenderer<TCanvas>(_roots: Map<TCanvas, Root>, _getEventPriority?:
// Since disposal is recursive, we can check the optional dispose arg, which will be undefined
// when the reconciler calls it, but then carry our own check recursively
const isPrimitive = child.__r3f?.primitive
const shouldDispose = dispose === undefined ? child.dispose !== null && !isPrimitive : dispose
const shouldDispose = !isPrimitive && (dispose === undefined ? child.dispose !== null : dispose)

// Remove nested child objects. Primitives should not have objects and children that are
// attached to them declaratively ...
18 changes: 18 additions & 0 deletions packages/fiber/tests/core/renderer.test.tsx
Original file line number Diff line number Diff line change
@@ -962,4 +962,22 @@ describe('renderer', () => {
expect(ref.current!.children).toStrictEqual([child1, child.current])
expect(ref.current!.userData.attach).toBe(attachedChild.current)
})

// TODO: scheduler isn't flushed during testing which prevents disposal
it.skip('should not recursively dispose of attached primitives', async () => {
const meshDispose = jest.fn()
const primitiveDispose = jest.fn()

await act(async () =>
root.render(
<mesh dispose={meshDispose}>
<primitive dispose={primitiveDispose} object={new THREE.BufferGeometry()} attach="geometry" />
</mesh>,
),
)
await act(async () => root.render(null))

expect(meshDispose).toBeCalledTimes(1)
expect(primitiveDispose).not.toBeCalled()
})
})