Fix/dropdown input result callbacks (#1071)

* fix uiDropdown not being updated

* fix tests

* fix test COVERAGE

* remove unnecessary return

* wip test

Author: gonpombo8

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

@@ -1,4 +1,12 @@
-import { Entity, IEngine, InputAction, PointerEventsSystem, PointerEventType } from '@dcl/ecs'
+import {
+  Entity,
+  IEngine,
+  InputAction,
+  PointerEventsSystem,
+  PointerEventType,
+  PBUiInputResult,
+  PBUiDropdownResult
+} from '@dcl/ecs'
 import * as components from '@dcl/ecs/dist/components'
 import Reconciler, { HostConfig } from 'react-reconciler'
 import { Callback, isListener, Listeners } from '../components'
@@ -183,6 +191,7 @@ export function createReconciler(
 
   function removeChildEntity(instance: Instance) {
     changeEvents.delete(instance.entity)
+    clickEvents.delete(instance.entity)
     engine.removeEntity(instance.entity)
     for (const child of instance._child) {
       removeChildEntity(child)
@@ -233,13 +242,24 @@ export function createReconciler(
   }
 
   function updateOnChange(entity: Entity, componentId: number, state?: OnChangeState) {
+    const hasEvent = changeEvents.has(entity)
     const event = changeEvents.get(entity) || changeEvents.set(entity, new Map()).get(entity)!
-    const oldState = event.get(componentId)
     const onChangeCallback = state?.onChangeCallback
     const onSubmitCallback = state?.onSubmitCallback
-    const value = state?.value ?? oldState?.value
-    const isSubmit = state?.isSubmit ?? oldState?.isSubmit
-    event.set(componentId, { onChangeCallback, onSubmitCallback, value, isSubmit })
+    event.set(componentId, { onChangeCallback, onSubmitCallback })
+    // Create onChange callback if its the first callback event for this entity
+    if (!hasEvent) {
+      const resultComponentId =
+        componentId === UiDropdown.componentId ? UiDropdownResult.componentId : UiInputResult.componentId
+      engine.getComponent<PBUiInputResult | PBUiDropdownResult>(resultComponentId).onChange(entity, (value) => {
+        if ((value as PBUiInputResult)?.isSubmit) {
+          const onSubmit = changeEvents.get(entity)?.get(componentId)?.onSubmitCallback
+          onSubmit && onSubmit(value?.value)
+        }
+        const onChange = changeEvents.get(entity)?.get(componentId)?.onChangeCallback
+        onChange && onChange(value?.value)
+      })
+    }
   }
 
   const hostConfig: HostConfig<
@@ -352,34 +372,8 @@ export function createReconciler(
     null
   )
 
-  // Maybe this could be something similar to Input system, but since we
-  // are going to use this only here, i prefer to scope it here.
-  function handleOnChange(componentId: number, resultComponent: typeof UiDropdownResult | typeof UiInputResult) {
-    for (const [entity, Result] of engine.getEntitiesWith(resultComponent)) {
-      const entityState = changeEvents.get(entity)?.get(componentId)
-      const isSubmit = !!(Result as any).isSubmit
-      if (entityState?.onChangeCallback && Result.value !== entityState.value) {
-        entityState.onChangeCallback(Result.value)
-      }
-      if (entityState?.onSubmitCallback && isSubmit && !entityState.isSubmit) {
-        entityState.onSubmitCallback(Result.value)
-      }
-
-      updateOnChange(entity, componentId, {
-        onChangeCallback: entityState?.onChangeCallback,
-        onSubmitCallback: entityState?.onSubmitCallback,
-        value: Result.value,
-        isSubmit
-      })
-    }
-  }
-
   return {
     update: function (component: ReactEcs.JSX.ReactNode) {
-      if (changeEvents.size) {
-        handleOnChange(UiInput.componentId, UiInputResult)
-        handleOnChange(UiDropdown.componentId, UiDropdownResult)
-      }
       return reconciler.updateContainer(component as any, root, null)
     },
     getEntities: () => Array.from(entities)

packages/@dcl/sdk/src/internal/transports/rendererTransport.ts

@@ -39,7 +39,6 @@ export function createRendererTransport(engineApi: EngineApiForTransport): Trans
       ) {
         return false
       }
-
       return !!message
     },
     type: 'renderer'

test/react-ecs/dropdown.spec.tsx

@@ -67,8 +67,9 @@ describe('UiDropdown React ECS', () => {
   })
 
   it('remove the onchange fn, so if we change the value there is no call to the fn because there is no onChange fn', async () => {
-    expect(onChange).toBeCalledTimes(0)
+    onChange.mockClear()
     conditional = false
+    expect(onChange).toBeCalledTimes(0)
     await engine.update(1)
 
     UiDropdownResult.getMutable(uiEntity).value = 2
@@ -78,6 +79,7 @@ describe('UiDropdown React ECS', () => {
   })
 
   it('put the conditional en true, so we have the onChange fn again', async () => {
+    onChange.mockClear()
     conditional = true
     await engine.update(1)
     expect(onChange).toBeCalledTimes(0)

test/react-ecs/input.spec.tsx

@@ -68,6 +68,7 @@ describe('Ui Listeners React Ecs', () => {
     UiInputResult.getMutable(uiEntity).isSubmit = true
     await engine.update(1)
     expect(onSubmit).toBeCalledTimes(1)
+    onChange.mockClear()
     removeComponent = true
     await engine.update(1)
     UiInputResult.create(uiEntity, { value: 'BOEDO' })

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

@@ -1,4 +1,4 @@
-SCENE_COMPILED_JS_SIZE_PROD=353.8k bytes
+SCENE_COMPILED_JS_SIZE_PROD=353.6k bytes
 (start empty vm 0.21.0-3680274614.commit-1808aa1)
   OPCODES ~= 0k
   MALLOC_COUNT = 1005
@@ -9,7 +9,7 @@ EVAL test/snapshots/production-bundles/ui.js
   REQUIRE: ~system/EngineApi
   REQUIRE: ~system/EngineApi
   OPCODES ~= 65k
-  MALLOC_COUNT = 19507
+  MALLOC_COUNT = 19502
   ALIVE_OBJS_DELTA ~= 3.88k
 CALL onStart()
   OPCODES ~= 0k
@@ -48,11 +48,11 @@ CALL onUpdate(0)
   Scene: PUT_COMPONENT e=0x209 c=1093 t=1 data={"placeholder":"","disabled":false}
   Scene: PUT_COMPONENT e=0x201 c=1094 t=1 data={"acceptEmpty":false,"options":["BOEDO","CASLA"],"selectedIndex":0,"disabled":false,"color":{"r":1,"g":0,"b":0,"a":1}}
   OPCODES ~= 134k
-  MALLOC_COUNT = 678
-  ALIVE_OBJS_DELTA ~= 0.24k
+  MALLOC_COUNT = 705
+  ALIVE_OBJS_DELTA ~= 0.25k
 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 ~= 66k
+  OPCODES ~= 65k
   MALLOC_COUNT = 250
   ALIVE_OBJS_DELTA ~= 0.12k
 CALL onUpdate(0.1)
@@ -65,4 +65,4 @@ CALL onUpdate(0.1)
   OPCODES ~= 64k
   MALLOC_COUNT = 0
   ALIVE_OBJS_DELTA ~= 0.00k
-  MEMORY_USAGE_COUNT ~= 1695.75k bytes
+  MEMORY_USAGE_COUNT ~= 1695.42k bytes