fix(react-ecs): don't drop an optional prop set for the first time (#1422)

propsChanged diffed only the keys present in prevProps, so an optional
prop that appears for the first time (absent from prevProps, present in
nextProps) was silently dropped on its first change.

Add a second loop over nextProps for keys not in prevProps. This is
allocation-free (no Object.keys/Set) to keep this per-frame hot path
cheap, and adds a propsChanged unit test covering the first-appearance
case.

Author: robtfm

packages/@dcl/react-ecs/src/reconciler/utils.ts

@@ -38,12 +38,25 @@ export function propsChanged<K extends keyof EntityComponents>(
   }
 
   const changes: Partial<EntityComponents[K]> = {}
+  // Iterate prevProps (this also catches removed keys, where nextProps[k] is undefined)...
   for (const k in prevProps) {
     const propKey = k as keyof typeof prevProps
     if (!isEqual(prevProps[propKey], nextProps[propKey])) {
       changes[propKey] = nextProps[propKey]
     }
   }
+  // ...then catch keys present only in nextProps (an optional prop set for the first time,
+  // e.g. an undefined value becoming defined). Iterating prevProps alone would drop these.
+  // Two allocation-free loops are used instead of a Set union to keep this hot path cheap.
+  // These keys aren't in prevProps, so emitting them whenever they're defined is both cheaper
+  // than (and avoids the falsy-value trap of) an isEqual(undefined, ...) comparison.
+  for (const k in nextProps) {
+    if (k in prevProps) continue
+    const propKey = k as keyof typeof nextProps
+    if (nextProps[propKey] !== undefined) {
+      changes[propKey] = nextProps[propKey]
+    }
+  }
 
   if (!Object.keys(changes).length) {
     return

test/react-ecs/reconciler-utils.spec.ts

@@ -0,0 +1,27 @@
+import { propsChanged } from '../../packages/@dcl/react-ecs/src/reconciler/utils'
+
+describe('reconciler propsChanged', () => {
+  it('detects a key present only in nextProps (optional prop set for the first time)', () => {
+    // Regression: the diff used to iterate only the keys present in prevProps, so a field
+    // appearing for the first time (absent from prevProps, present in nextProps) was silently
+    // dropped on its first change. This is what made e.g. the first scroll-to-target — and any
+    // optional prop going from undefined to a value — never reach the engine.
+    expect(propsChanged('uiTransform', {}, { width: 100 })).toEqual({
+      type: 'put',
+      component: 'uiTransform',
+      props: { width: 100 }
+    })
+  })
+
+  it('still detects a changed existing key', () => {
+    expect(propsChanged('uiTransform', { width: 100 }, { width: 200 })).toEqual({
+      type: 'put',
+      component: 'uiTransform',
+      props: { width: 200 }
+    })
+  })
+
+  it('returns undefined when nothing changed', () => {
+    expect(propsChanged('uiTransform', { width: 100 }, { width: 100 })).toBeUndefined()
+  })
+})

test/snapshots/production-bundles/ui.ts.crdt

@@ -53,17 +53,17 @@ CALL onUpdate(0)
   ALIVE_OBJS_DELTA ~= 0.31k
 CALL onUpdate(0.1)
   Scene: PUT_COMPONENT e=0x200 c=1 t=2 data={"position":{"x":8,"y":1,"z":8},"rotation":{"x":0,"y":0.008726535364985466,"z":0,"w":0.9999619126319885},"scale":{"x":1,"y":1,"z":1},"parent":0}
-  OPCODES ~= 70k
+  OPCODES ~= 76k
   MALLOC_COUNT = 259
   ALIVE_OBJS_DELTA ~= 0.13k
 CALL onUpdate(0.1)
   Scene: PUT_COMPONENT e=0x200 c=1 t=3 data={"position":{"x":8,"y":1,"z":8},"rotation":{"x":0,"y":0.017452405765652657,"z":0,"w":0.9998477101325989},"scale":{"x":1,"y":1,"z":1},"parent":0}
-  OPCODES ~= 69k
+  OPCODES ~= 74k
   MALLOC_COUNT = 35
   ALIVE_OBJS_DELTA ~= 0.02k
 CALL onUpdate(0.1)
   Scene: PUT_COMPONENT e=0x200 c=1 t=4 data={"position":{"x":8,"y":1,"z":8},"rotation":{"x":0,"y":0.026176948100328445,"z":0,"w":0.9996573328971863},"scale":{"x":1,"y":1,"z":1},"parent":0}
-  OPCODES ~= 69k
+  OPCODES ~= 74k
   MALLOC_COUNT = 0
   ALIVE_OBJS_DELTA ~= 0.00k
-  MEMORY_USAGE_COUNT ~= 1889.04k bytes
+  MEMORY_USAGE_COUNT ~= 1889.20k bytes