Skip to content

Commit 6f2f523

Browse files
committed
fix(EffectComposer): drop dedupeByLastOccurrence, fixed upstream in r3f 9.7.0
r3f 9.7.0 fixes the reconciler bug this workaround existed for: internal instance children were drifting out of sync on a keyed reorder (reported upstream by us, fixed in pmndrs/react-three-fiber#3808). With the fix in place, group.current.__r3f.children no longer contains a stale duplicate after React moves a keyed child, so the dedupe step is no longer needed. Bumped @react-three/fiber to ^9.7.0 and removed dedupeByLastOccurrence, reading groupInstance.children directly. Verified empirically: the existing reorder-with-key test passes on 9.7.0 without the workaround, and fails with the exact same symptom as before (Expected EffectPass with 2 effects) when temporarily downgraded back to 9.6.1 with the workaround still removed - confirming this is the actual upstream fix taking effect, not a coincidence.
1 parent 76b09e5 commit 6f2f523

3 files changed

Lines changed: 196 additions & 42 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
"@eslint/compat": "^2.1.0",
3939
"@eslint/eslintrc": "^3.3.6",
4040
"@eslint/js": "^9.39.5",
41-
"@react-three/fiber": "^9.6.1",
41+
"@react-three/fiber": "^9.7.0",
4242
"@types/node": "^26.1.1",
4343
"@types/react": "^19.2.17",
4444
"@types/three": "^0.182.0",

src/EffectComposer.tsx

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,6 @@ type ComposerState = {
5959
const isConvolution = (effect: Effect): boolean =>
6060
(effect.getAttributes() & EffectAttribute.CONVOLUTION) === EffectAttribute.CONVOLUTION
6161

62-
/**
63-
* r3f's host config doesn't detach an existing instance from its old slot
64-
* when React moves it (a key-preserving reorder of siblings) — it just
65-
* splices/pushes the moved instance into its new slot, leaving a stale
66-
* duplicate of the same object behind at the old one. Deduping by last
67-
* occurrence recovers the correct order regardless: the stale copy is
68-
* always left behind first, the moved one lands later.
69-
*/
70-
function dedupeByLastOccurrence(children: Instance<Group>['children']): unknown[] {
71-
const lastIndex = new Map<unknown, number>()
72-
children.forEach((child, index) => lastIndex.set(child.object, index))
73-
return Array.from(lastIndex.entries())
74-
.sort(([, a], [, b]) => a - b)
75-
.map(([object]) => object)
76-
}
77-
7862
/**
7963
* Groups a flat, ordered list of Effect/Pass instances into actual composer
8064
* passes, merging consecutive non-convolution Effects into a single
@@ -192,7 +176,7 @@ export const EffectComposer = /* @__PURE__ */ memo(function EffectComposer({
192176
const groupInstance = (group.current as Group & { __r3f: Instance<Group> }).__r3f
193177

194178
if (groupInstance) {
195-
const nodes = dedupeByLastOccurrence(groupInstance.children).filter(
179+
const nodes = groupInstance.children.map((child) => child.object).filter(
196180
(object): object is Effect | Pass => object instanceof Effect || object instanceof Pass
197181
)
198182

0 commit comments

Comments
 (0)