diff --git a/src/Selection.tsx b/src/Selection.tsx index 7151a9f..72668d1 100644 --- a/src/Selection.tsx +++ b/src/Selection.tsx @@ -1,7 +1,7 @@ import { type ThreeElements } from '@react-three/fiber' import { createContext, - useContext, + use, useEffect, useMemo, useRef, @@ -10,7 +10,7 @@ import { type ReactNode, type SetStateAction, } from 'react' -import { type Group, type Object3D } from 'three' +import { type Group, type Line, type Mesh, type Object3D, type Points } from 'three' export type Api = { selected: Object3D[] @@ -29,25 +29,55 @@ export function Selection({ children, enabled = true }: { enabled?: boolean; chi return {children} } +// Covers Mesh/Line/Points subclasses too, unlike `.type`. +function isSelectable(object: Object3D): boolean { + const o = object as Partial + return !!(o.isMesh || o.isLine || o.isPoints) +} + export function Select({ enabled = false, children, ...props }: SelectApi) { const group = useRef(null!) - const api = useContext(selectionContext) + // Stable, unlike the context value - avoids retriggering off our own write. + const select = use(selectionContext)?.select + const claimed = useRef([]) + useEffect(() => { - if (api && enabled) { - let changed = false - const current: Object3D[] = [] + if (!select) return + + const current: Object3D[] = [] + if (enabled) { group.current.traverse((o) => { - o.type === 'Mesh' && current.push(o) - if (api.selected.indexOf(o) === -1) changed = true + if (isSelectable(o)) current.push(o) }) - if (changed) { - api.select((state) => [...state, ...current]) - return () => { - api.select((state) => state.filter((selected) => !current.includes(selected))) - } - } } - }, [enabled, children, api]) + + const previouslyClaimed = claimed.current + claimed.current = current + + select((prev) => { + const prevSet = new Set(prev) + const currentSet = new Set(current) + const toAdd = current.filter((o) => !prevSet.has(o)) + const toRemove = previouslyClaimed.filter((o) => !currentSet.has(o) && prevSet.has(o)) + if (!toAdd.length && !toRemove.length) return prev + const toRemoveSet = toRemove.length ? new Set(toRemove) : null + const kept = toRemoveSet ? prev.filter((o) => !toRemoveSet.has(o)) : prev + return toAdd.length ? [...kept, ...toAdd] : kept + }) + }, [enabled, children, select]) + + // Separate from the effect above so unmount cleanup doesn't fire on every enabled/children change. + useEffect(() => { + return () => { + if (!select || !claimed.current.length) return + const stillClaimed = new Set(claimed.current) + select((prev) => { + const next = prev.filter((o) => !stillClaimed.has(o)) + return next.length !== prev.length ? next : prev + }) + } + }, [select]) + return ( {children} diff --git a/src/tests/Selection.test.tsx b/src/tests/Selection.test.tsx new file mode 100644 index 0000000..cdc9dc4 --- /dev/null +++ b/src/tests/Selection.test.tsx @@ -0,0 +1,324 @@ +import * as React from 'react' +import * as THREE from 'three' +import { afterEach, describe, expect, it } from 'vitest' +import { Select, Selection, selectionContext } from '../Selection' +import { flush, root } from './test-utils' + +afterEach(async () => { + await React.act(async () => { + root.render(null) + }) +}) + +function Capture({ onSnapshot }: { onSnapshot: (selected: THREE.Object3D[]) => void }) { + const api = React.useContext(selectionContext) + React.useEffect(() => { + onSnapshot(api?.selected ?? []) + }) + return null +} + +describe('Selection/Select', () => { + it('settles to a stable selected array without looping', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + await React.act(async () => + root.render( + + snapshots.push(s)} /> + + + ) + ) + await flush() + await flush() + + // Exactly one render for the initial (empty) commit, one for the + // single necessary addition - the original bug kept re-triggering + // itself, so this would grow unbounded (and eventually throw) instead + // of stopping at 2. + expect(snapshots).toEqual([[], [meshRef.current]]) + }) + + it('selects a Line and a Points object, not just Mesh', async () => { + const snapshots: THREE.Object3D[][] = [] + const lineRef = React.createRef() + const pointsRef = React.createRef() + + await React.act(async () => + root.render( + + snapshots.push(s)} /> + + + + ) + ) + await flush() + await flush() + + const last = snapshots[snapshots.length - 1] + expect(last).toContain(lineRef.current) + expect(last).toContain(pointsRef.current) + }) + + it('removes its objects once unmounted, leaving a sibling Select untouched', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshARef = React.createRef() + const meshBRef = React.createRef() + + const render = (mountA: boolean) => + root.render( + + snapshots.push(s)} /> + {mountA && ( + + )} + + + ) + + await React.act(async () => render(true)) + await flush() + await flush() + expect(snapshots[snapshots.length - 1]).toEqual(expect.arrayContaining([meshARef.current, meshBRef.current])) + + await React.act(async () => render(false)) + await flush() + await flush() + + const last = snapshots[snapshots.length - 1] + expect(last).not.toContain(meshARef.current) + expect(last).toContain(meshBRef.current) + }) + + it('removes its objects when enabled toggles to false', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + const render = (enabled: boolean) => + root.render( + + snapshots.push(s)} /> + + + ) + + await React.act(async () => render(true)) + await flush() + await flush() + expect(snapshots[snapshots.length - 1]).toContain(meshRef.current) + + await React.act(async () => render(false)) + await flush() + await flush() + expect(snapshots[snapshots.length - 1]).not.toContain(meshRef.current) + }) + + it('selects a SkinnedMesh - isMesh is inherited, not just the exact type string', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + await React.act(async () => + root.render( + + snapshots.push(s)} /> + + + ) + ) + await flush() + await flush() + + expect(snapshots[snapshots.length - 1]).toContain(meshRef.current) + }) + + it('does not duplicate an object claimed by nested Selects', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + await React.act(async () => + root.render( + + snapshots.push(s)} /> + + + + + + + + + ) + ) + await flush() + await flush() + await flush() + + const last = snapshots[snapshots.length - 1] + expect(last.filter((o) => o === meshRef.current)).toHaveLength(1) + }) + + it('keeps an object selected via the outer Select after the inner one disables', async () => { + const snapshots: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + const render = (innerEnabled: boolean) => + root.render( + + snapshots.push(s)} /> + + + + + + + + + ) + + await React.act(async () => render(true)) + await flush() + await flush() + await flush() + expect(snapshots[snapshots.length - 1]).toContain(meshRef.current) + + await React.act(async () => render(false)) + await flush() + await flush() + await flush() + + // The inner Select's own cleanup drops its claim, but the mesh never + // left the outer Select's subtree - its own traversal still finds it. + expect(snapshots[snapshots.length - 1]).toContain(meshRef.current) + }) + + it('does not change the selected array reference on a re-render where children only changes identity', async () => { + const refs: THREE.Object3D[][] = [] + const meshRef = React.createRef() + + function CaptureRef() { + const api = React.useContext(selectionContext) + React.useEffect(() => { + if (api) refs.push(api.selected) + }) + return null + } + + // A fresh JSX tree every call, like a parent re-rendering for an + // unrelated reason - Select's own `children` prop is a new reference + // each time even though the underlying mesh never changes. + const render = () => + root.render( + + + + + ) + + await React.act(async () => render()) + await flush() + await flush() + + refs.length = 0 + for (let i = 0; i < 5; i++) { + await React.act(async () => render()) + await flush() + } + + expect(new Set(refs).size).toBe(1) + }) + + it('a cleanup updater returns the same reference when nothing is left for it to remove', async () => { + const meshRef = React.createRef() + const updaters: Array<(prev: THREE.Object3D[]) => THREE.Object3D[]> = [] + + // React batches both nested Selects' cleanup calls into a single + // re-render regardless of this bail-out (the net change from mount to + // unmount is real), so the optimization isn't observable through + // rendering alone. Capture the raw updater functions instead and + // replay them directly to verify the second one bails. + function CapturingSelection({ children }: { children: React.ReactNode }) { + const [selected, setSelected] = React.useState([]) + const select = React.useCallback((updater: React.SetStateAction) => { + if (typeof updater === 'function') updaters.push(updater as (prev: THREE.Object3D[]) => THREE.Object3D[]) + setSelected(updater) + }, []) + const value = React.useMemo(() => ({ selected, select, enabled: true }), [selected, select]) + return {children} + } + + const render = (mounted: boolean) => + root.render( + + {mounted && ( + + + + + + + + )} + + ) + + await React.act(async () => render(true)) + await flush() + await flush() + await flush() + + const mesh = meshRef.current! + updaters.length = 0 + await React.act(async () => render(false)) + await flush() + await flush() + await flush() + + expect(updaters).toHaveLength(2) + const afterFirst = updaters[0]([mesh]) + const afterSecond = updaters[1](afterFirst) + expect(afterSecond).toBe(afterFirst) + }) +})