From e2d36950e6950e77c652b70098611f331e32d728 Mon Sep 17 00:00:00 2001
From: Brian Zinn <github@wakeskate.com>
Date: Fri, 4 Jun 2021 20:56:44 -0700
Subject: [PATCH] fix Observable<T> add/remove infinite loop by setting
 Observer<T> callback property.

---
 src/UpdateInstance.ts                         | 13 ++++---
 stories/babylonjs/Basic/toggleMesh.stories.js | 38 +++++++++----------
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/UpdateInstance.ts b/src/UpdateInstance.ts
index 23cb1949..ffcd58b2 100644
--- a/src/UpdateInstance.ts
+++ b/src/UpdateInstance.ts
@@ -46,11 +46,14 @@ export const applyUpdateToInstance = (createdInstance: CreatedInstance<any>, upd
       target[update.propertyName] = update.value;
       break;
     case PropChangeType.Observable:
-      const observer: Nullable<Observer<any>> = (target[update.propertyName] as Observable<any>).add(update.value);
-      if(update.propertyName in createdInstance.observers) {
-        (target[update.propertyName] as Observable<any>).remove(createdInstance.observers[update.propertyName]);
+      // adding and remove observer can cause an infinite loop without something like  setTimeout(() => ..., 1);
+      // ie: clicking a button causes a re-render, which causes the callback to change (inline method)...
+      if (update.propertyName in createdInstance.observers) {
+        createdInstance.observers[update.propertyName]!.callback = update.value;
+      } else {
+        const observer: Nullable<Observer<any>> = (target[update.propertyName] as Observable<any>).add(update.value);
+        createdInstance.observers[update.propertyName] = observer;
       }
-      createdInstance.observers[update.propertyName] = observer;
       break;
     case PropChangeType.Method:
       if (typeof target[update.propertyName] === "function") {
@@ -124,7 +127,7 @@ export const applyInitialPropsToCreatedInstance = (createdInstance: CreatedInsta
  */
 export const applyInitialPropsToInstance = (hostInstance: any, props: any): void => {
   // this is a bad cast.  it is here for backwards compatibility with a react-spring dependency that only uses vector/color prop changes.
-  applyPropsToRef({hostInstance} as CreatedInstance<any>, props);
+  applyPropsToRef({ hostInstance } as CreatedInstance<any>, props);
 }
 
 /**
diff --git a/stories/babylonjs/Basic/toggleMesh.stories.js b/stories/babylonjs/Basic/toggleMesh.stories.js
index 5731da9a..993d649c 100644
--- a/stories/babylonjs/Basic/toggleMesh.stories.js
+++ b/stories/babylonjs/Basic/toggleMesh.stories.js
@@ -1,8 +1,8 @@
-import React, { useState, useCallback } from 'react'
-import {Engine, Scene} from '../../../dist/react-babylonjs'
-import {Vector3} from '@babylonjs/core/Maths/math'
+import React, { useState } from 'react'
+import { Engine, Scene } from '../../../dist/react-babylonjs'
+import { Vector3 } from '@babylonjs/core/Maths/math'
 import '../../style.css'
-import {Color3} from "@babylonjs/core";
+import { Color3 } from "@babylonjs/core";
 import ScaledModelWithProgress from "../ScaledModelWithProgress";
 
 export default { title: 'Babylon Basic' };
@@ -19,37 +19,37 @@ export default { title: 'Babylon Basic' };
 function WithToggleMesh() {
   const [isShown, toggleShown] = useState(true);
 
-  const onClick = useCallback(() => {
-    console.log('toggle clicked');
+  const onClick = () => {
+    console.log('toggle clicked:', isShown);
     toggleShown(isShown => !isShown)
-  }, []);
+  };
 
   return (
     <>
       {isShown &&
         <ground name='ground1' width={10} height={6} subdivisions={2}
-                position={new Vector3(0, 0, 0)}>
+          position={new Vector3(0, 0, 0)}>
 
           <sphere name='sphere1' diameter={2} segments={16}
-                  position={new Vector3(-2, 1, 0)}>
+            position={new Vector3(-2, 1, 0)}>
           </sphere>
 
           <ScaledModelWithProgress
             rootUrl={`assets/models/`} sceneFilename='atom.glb'
             progressBarColor={Color3.FromInts(135, 206, 235)}
-            center={new Vector3(2, 1, 0)}/>
+            center={new Vector3(2, 1, 0)} />
         </ground>
       }
 
       <adtFullscreenUi name='ui'>
         <rectangle name="rect" height='50px' width='150px'>
           <rectangle>
-            <babylon-button name="close-icon"
-                            background="green"
-                            onPointerDownObservable={onClick}>
+            <babylon-button name="show-hide-button"
+              background="green"
+              onPointerDownObservable={onClick}>
               <textBlock text={isShown ? 'hide' : 'show'}
-                         fontFamily="FontAwesome" fontStyle="bold"
-                         fontSize={20} color="white"/>
+                fontFamily="FontAwesome" fontStyle="bold"
+                fontSize={20} color="white" />
             </babylon-button>
           </rectangle>
         </rectangle>
@@ -59,13 +59,13 @@ function WithToggleMesh() {
 }
 
 export const ToggleMesh = () => (
-  <div style={{flex: 1, display: 'flex'}}>
+  <div style={{ flex: 1, display: 'flex' }}>
     <Engine antialias adaptToDeviceRatio canvasId='babylonJS'>
       <Scene>
         <freeCamera name='camera1' position={new Vector3(0, 5, -10)}
-                    setTarget={[Vector3.Zero()]}/>
-        <hemisphericLight name='light1' intensity={0.7} direction={Vector3.Up()}/>
-        <WithToggleMesh/>
+          setTarget={[Vector3.Zero()]} />
+        <hemisphericLight name='light1' intensity={0.7} direction={Vector3.Up()} />
+        <WithToggleMesh />
       </Scene>
     </Engine>
   </div>