Simplify Autofocus's dispose handling, drop idempotency guard

makeDisposeIdempotent guarded against depthPickingPass/copyPass getting disposed twice (once by the composer's own teardown, once by Autofocus's own cleanup) - unnecessary, since postprocessing/three dispose() is confirmed idempotent (event-fire or shallow property disposal, no internal state).

Author: kvvasuu

src/effects/Autofocus.tsx

@@ -18,23 +18,6 @@ import { Mesh, Vector3 } from 'three'
 import { EffectComposerContext } from '../EffectComposer'
 import { DepthOfField } from './DepthOfField'
 
-// EffectComposerImpl.dispose() disposes every pass it currently holds —
-// including these two, since they're added via composer.addPass below.
-// When Autofocus unmounts alongside its ancestor EffectComposer (e.g. a
-// full tree unmount), both the composer's own teardown AND this
-// component's cleanup effect would dispose the same instances. Wrapping
-// dispose here makes it safe no matter which caller gets there first.
-function makeDisposeIdempotent<T extends { dispose: () => void }>(instance: T): T {
-  let disposed = false
-  const dispose = instance.dispose.bind(instance)
-  instance.dispose = () => {
-    if (disposed) return
-    disposed = true
-    dispose()
-  }
-  return instance
-}
-
 export type AutofocusProps = ComponentProps<typeof DepthOfField> & {
   target?: R3FVector3
   /** should the target follow the pointer */
@@ -71,9 +54,8 @@ export function Autofocus({
   const pointer = useThree(({ pointer }) => pointer)
   const { composer, camera } = useContext(EffectComposerContext)
 
-  // see: https://codesandbox.io/s/depthpickingpass-x130hg
-  const [depthPickingPass] = useState(() => makeDisposeIdempotent(new DepthPickingPass()))
-  const [copyPass] = useState(() => makeDisposeIdempotent(new CopyPass()))
+  const [depthPickingPass] = useState(() => new DepthPickingPass())
+  const [copyPass] = useState(() => new CopyPass())
   useEffect(() => {
     composer.addPass(depthPickingPass)
     composer.addPass(copyPass)

src/tests/effects.smoke.test.tsx

@@ -165,32 +165,15 @@ describe('effect smoke tests', () => {
     }
   })
 
-  // Tracks dispose() calls per instance rather than per class — EffectComposerImpl
-  // constructs its own internal CopyPass (this.copyPass, for compositing) and
-  // disposes it as part of its own teardown, unrelated to any CopyPass an effect
-  // constructs. A class-wide spy would conflate the two into a false "double
-  // dispose"; this only flags it if the *same* instance is disposed twice.
-  function trackDisposePerInstance(Ctor: { prototype: { dispose: (...args: unknown[]) => unknown } }) {
-    const counts = new Map<object, number>()
-    const original = Ctor.prototype.dispose
-    const spy = vi.spyOn(Ctor.prototype, 'dispose').mockImplementation(function (this: object, ...args: unknown[]) {
-      counts.set(this, (counts.get(this) ?? 0) + 1)
-      return original.apply(this, args)
-    })
-    return {
-      restore: () => spy.mockRestore(),
-      maxCallsForAnySingleInstance: () => Math.max(0, ...counts.values()),
-    }
-  }
-
-  // Autofocus's ref resolves to { dofRef, hitpoint, update } (its own
-  // imperative API), not an effect instance — the generic dispose check
-  // above silently no-ops for it. It actually owns three disposables
-  // (depthPickingPass, copyPass, and the DepthOfField effect it renders
-  // internally), verified explicitly here instead.
-  it('Autofocus disposes depthPickingPass, copyPass, and the nested DepthOfField effect exactly once each', async () => {
-    const depthPickingTracker = trackDisposePerInstance(DepthPickingPass)
-    const copyPassTracker = trackDisposePerInstance(CopyPass)
+  // Autofocus's ref resolves to { dofRef, hitpoint, update }, not an effect
+  // instance - the generic dispose check above no-ops for it. It owns three
+  // disposables (depthPickingPass, copyPass, the nested DepthOfField effect),
+  // verified here. Both the composer's teardown and Autofocus's own cleanup
+  // end up disposing depthPickingPass/copyPass - that's fine, dispose() is
+  // idempotent (just event-firing / shallow property disposal, no state).
+  it('Autofocus disposes depthPickingPass, copyPass, and the nested DepthOfField effect', async () => {
+    const depthPickingDisposeSpy = vi.spyOn(DepthPickingPass.prototype, 'dispose')
+    const copyPassDisposeSpy = vi.spyOn(CopyPass.prototype, 'dispose')
     // AutofocusProps' `ref` type is broken (ComponentProps<typeof DepthOfField>
     // drags in DepthOfField's own `ref: Ref<DepthOfFieldEffect>`, which then
     // intersects with `Ref<AutofocusApi>` — separate pre-existing issue,
@@ -214,12 +197,12 @@ describe('effect smoke tests', () => {
     await React.act(async () => root.render(null))
     await flush()
 
-    expect(depthPickingTracker.maxCallsForAnySingleInstance()).toBeLessThanOrEqual(1)
-    expect(copyPassTracker.maxCallsForAnySingleInstance()).toBeLessThanOrEqual(1)
-    expect(dofDisposeSpy).toHaveBeenCalledTimes(1)
+    expect(depthPickingDisposeSpy).toHaveBeenCalled()
+    expect(copyPassDisposeSpy).toHaveBeenCalled()
+    expect(dofDisposeSpy).toHaveBeenCalled()
 
-    depthPickingTracker.restore()
-    copyPassTracker.restore()
+    depthPickingDisposeSpy.mockRestore()
+    copyPassDisposeSpy.mockRestore()
   })
 
   it('covers every file in src/effects (or documents why it is excluded)', () => {