fix(Selection): stop Select's effect from depending on its own write

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,56 @@ export function Selection({ children, enabled = true }: { enabled?: boolean; chi
   return <selectionContext.Provider value={value}>{children}</selectionContext.Provider>
 }
 
+// `.type` is a free-form string; these flags cover Mesh/Line/Points subclasses too.
+function isSelectable(object: Object3D): boolean {
+  const o = object as Partial<Mesh & Line & Points>
+  return !!(o.isMesh || o.isLine || o.isPoints)
+}
+
 export function Select({ enabled = false, children, ...props }: SelectApi) {
   const group = useRef<Group>(null!)
-  const api = useContext(selectionContext)
+  // Stable setter, unlike the context value - avoids retriggering off our own write.
+  const select = use(selectionContext)?.select
+  // What this Select currently has claimed in `selected`, so a re-run
+  // diffs against that instead of unconditionally clearing and re-adding through effect cleanup.
+  const claimed = useRef<Object3D[]>([])
+
   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) => {
+      // Add anything missing from live state - covers both the normal
+      // case and re-asserting a claim some other Select's own update
+      // dropped in the meantime (nested Selects share objects). Only
+      // remove what this Select itself is no longer claiming.
+      const toAdd = current.filter((o) => !prev.includes(o))
+      const toRemove = previouslyClaimed.filter((o) => !current.includes(o) && prev.includes(o))
+      if (!toAdd.length && !toRemove.length) return prev
+      const kept = toRemove.length ? prev.filter((o) => !toRemove.includes(o)) : prev
+      return toAdd.length ? [...kept, ...toAdd] : kept
+    })
+  }, [enabled, children, select])
+
+  // Only for unmount - a separate effect so it doesn't fire on every
+  // enabled/children change like the diffing effect above does.
+  useEffect(() => {
+    return () => {
+      if (!select || !claimed.current.length) return
+      const stillClaimed = claimed.current
+      select((prev) => prev.filter((o) => !stillClaimed.includes(o)))
+    }
+  }, [select])
+
   return (
     <group ref={group} {...props}>
       {children}

src/tests/Selection.test.tsx

@@ -0,0 +1,271 @@
+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<THREE.Mesh>()
+
+    await React.act(async () =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled>
+            <mesh ref={meshRef}>
+              <boxGeometry />
+              <meshBasicMaterial />
+            </mesh>
+          </Select>
+        </Selection>
+      )
+    )
+    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<THREE.Line>()
+    const pointsRef = React.createRef<THREE.Points>()
+
+    await React.act(async () =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled>
+            <threeLine ref={lineRef}>
+              <bufferGeometry />
+              <lineBasicMaterial />
+            </threeLine>
+          </Select>
+          <Select enabled>
+            <points ref={pointsRef}>
+              <bufferGeometry />
+              <pointsMaterial />
+            </points>
+          </Select>
+        </Selection>
+      )
+    )
+    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<THREE.Mesh>()
+    const meshBRef = React.createRef<THREE.Mesh>()
+
+    const render = (mountA: boolean) =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          {mountA && (
+            <Select key="a" enabled>
+              <mesh ref={meshARef}>
+                <boxGeometry />
+                <meshBasicMaterial />
+              </mesh>
+            </Select>
+          )}
+          <Select key="b" enabled>
+            <mesh ref={meshBRef}>
+              <boxGeometry />
+              <meshBasicMaterial />
+            </mesh>
+          </Select>
+        </Selection>
+      )
+
+    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<THREE.Mesh>()
+
+    const render = (enabled: boolean) =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled={enabled}>
+            <mesh ref={meshRef}>
+              <boxGeometry />
+              <meshBasicMaterial />
+            </mesh>
+          </Select>
+        </Selection>
+      )
+
+    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<THREE.SkinnedMesh>()
+
+    await React.act(async () =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled>
+            <skinnedMesh ref={meshRef} />
+          </Select>
+        </Selection>
+      )
+    )
+    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<THREE.Mesh>()
+
+    await React.act(async () =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled>
+            <Select enabled>
+              <mesh ref={meshRef}>
+                <boxGeometry />
+                <meshBasicMaterial />
+              </mesh>
+            </Select>
+          </Select>
+        </Selection>
+      )
+    )
+    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<THREE.Mesh>()
+
+    const render = (innerEnabled: boolean) =>
+      root.render(
+        <Selection>
+          <Capture onSnapshot={(s) => snapshots.push(s)} />
+          <Select enabled>
+            <Select enabled={innerEnabled}>
+              <mesh ref={meshRef}>
+                <boxGeometry />
+                <meshBasicMaterial />
+              </mesh>
+            </Select>
+          </Select>
+        </Selection>
+      )
+
+    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<THREE.Mesh>()
+
+    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(
+        <Selection>
+          <CaptureRef />
+          <Select enabled>
+            <mesh ref={meshRef}>
+              <boxGeometry />
+              <meshBasicMaterial />
+            </mesh>
+          </Select>
+        </Selection>
+      )
+
+    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)
+  })
+})