Properly unregister listeners when React component unmount (#748)

aframevr · Aug 19, 2024 · 3e95a3b · 3e95a3b
1 parent 94184c4
commit 3e95a3b
Show file tree

Hide file tree

Showing 9 changed files with 134 additions and 66 deletions.
diff --git a/src/components/components/CommonComponents.js b/src/components/components/CommonComponents.js
@@ -28,18 +28,24 @@ export default class CommonComponents extends React.Component {
     entity: PropTypes.object
   };
 
+  onEntityUpdate = (detail) => {
+    if (detail.entity !== this.props.entity) {
+      return;
+    }
+    if (
+      DEFAULT_COMPONENTS.indexOf(detail.component) !== -1 ||
+      detail.component === 'mixin'
+    ) {
+      this.forceUpdate();
+    }
+  };
+
   componentDidMount() {
-    Events.on('entityupdate', (detail) => {
-      if (detail.entity !== this.props.entity) {
-        return;
-      }
-      if (
-        DEFAULT_COMPONENTS.indexOf(detail.component) !== -1 ||
-        detail.component === 'mixin'
-      ) {
-        this.forceUpdate();
-      }
-    });
+    Events.on('entityupdate', this.onEntityUpdate);
+  }
+
+  componentWillUnmount() {
+    Events.off('entityupdate', this.onEntityUpdate);
   }
 
   renderCommonAttributes() {

diff --git a/src/components/components/Component.js b/src/components/components/Component.js
@@ -29,15 +29,21 @@ export default class Component extends React.Component {
     };
   }
 
+  onEntityUpdate = (detail) => {
+    if (detail.entity !== this.props.entity) {
+      return;
+    }
+    if (detail.component === this.props.name) {
+      this.forceUpdate();
+    }
+  };
+
   componentDidMount() {
-    Events.on('entityupdate', (detail) => {
-      if (detail.entity !== this.props.entity) {
-        return;
-      }
-      if (detail.component === this.props.name) {
-        this.forceUpdate();
-      }
-    });
+    Events.on('entityupdate', this.onEntityUpdate);
+  }
+
+  componentWillUnmount() {
+    Events.off('entityupdate', this.onEntityUpdate);
   }
 
   static getDerivedStateFromProps(props, state) {

diff --git a/src/components/components/ComponentsContainer.js b/src/components/components/ComponentsContainer.js
@@ -11,15 +11,21 @@ export default class ComponentsContainer extends React.Component {
     entity: PropTypes.object
   };
 
+  onEntityUpdate = (detail) => {
+    if (detail.entity !== this.props.entity) {
+      return;
+    }
+    if (detail.component === 'mixin') {
+      this.forceUpdate();
+    }
+  };
+
   componentDidMount() {
-    Events.on('entityupdate', (detail) => {
-      if (detail.entity !== this.props.entity) {
-        return;
-      }
-      if (detail.component === 'mixin') {
-        this.forceUpdate();
-      }
-    });
+    Events.on('entityupdate', this.onEntityUpdate);
+  }
+
+  componentWillUnmount() {
+    Events.off('entityupdate', this.onEntityUpdate);
   }
 
   render() {

diff --git a/src/components/components/Sidebar.js b/src/components/components/Sidebar.js
@@ -14,14 +14,28 @@ export default class Sidebar extends React.Component {
     this.state = { open: false };
   }
 
+  onComponentRemove = (detail) => {
+    if (detail.entity !== this.props.entity) {
+      return;
+    }
+    this.forceUpdate();
+  };
+
+  onComponentAdd = (detail) => {
+    if (detail.entity !== this.props.entity) {
+      return;
+    }
+    this.forceUpdate();
+  };
+
   componentDidMount() {
-    Events.on('componentremove', (event) => {
-      this.forceUpdate();
-    });
+    Events.on('componentremove', this.onComponentRemove);
+    Events.on('componentadd', this.onComponentAdd);
+  }
 
-    Events.on('componentadd', (event) => {
-      this.forceUpdate();
-    });
+  componentWillUnmount() {
+    Events.off('componentremove', this.onComponentRemove);
+    Events.off('componentadd', this.onComponentAdd);
   }
 
   handleToggle = () => {

diff --git a/src/components/modals/ModalTextures.js b/src/components/modals/ModalTextures.js
@@ -65,14 +65,19 @@ export default class ModalTextures extends React.Component {
     this.registryGallery = React.createRef();
   }
 
-  componentDidMount() {
-    Events.on('assetsimagesload', (images) => {
-      this.generateFromRegistry();
-    });
+  onAssetsImagesLoad = (images) => {
+    this.generateFromRegistry();
+  };
 
+  componentDidMount() {
+    Events.on('assetsimagesload', this.onAssetsImagesLoad);
     this.generateFromAssets();
   }
 
+  componentWillUnmount() {
+    Events.off('assetsimagesload', this.onAssetsImagesLoad);
+  }
+
   componentDidUpdate(prevProps) {
     if (this.state.isOpen && !AFRAME.INSPECTOR.assetsLoader.hasLoaded) {
       AFRAME.INSPECTOR.assetsLoader.load();

diff --git a/src/components/scenegraph/SceneGraph.js b/src/components/scenegraph/SceneGraph.js
@@ -44,16 +44,25 @@ export default class SceneGraph extends React.Component {
     );
   }
 
+  onMixinUpdate = (detail) => {
+    if (detail.component === 'mixin') {
+      this.rebuildEntityOptions();
+    }
+  };
+
   componentDidMount() {
     this.rebuildEntityOptions();
     Events.on('entityidchange', this.rebuildEntityOptions);
     Events.on('entitycreated', this.rebuildEntityOptions);
     Events.on('entityclone', this.rebuildEntityOptions);
-    Events.on('entityupdate', (detail) => {
-      if (detail.component === 'mixin') {
-        this.rebuildEntityOptions();
-      }
-    });
+    Events.on('entityupdate', this.onMixinUpdate);
+  }
+
+  componentWillUnmount() {
+    Events.off('entityidchange', this.rebuildEntityOptions);
+    Events.off('entitycreated', this.rebuildEntityOptions);
+    Events.off('entityclone', this.rebuildEntityOptions);
+    Events.off('entityupdate', this.onMixinUpdate);
   }
 
   /**

diff --git a/src/components/viewport/CameraToolbar.js b/src/components/viewport/CameraToolbar.js
@@ -60,15 +60,21 @@ export default class CameraToolbar extends React.Component {
     this.justChangedCamera = false;
   }
 
+  onCameraToggle = (data) => {
+    if (this.justChangedCamera) {
+      // Prevent recursion.
+      this.justChangedCamera = false;
+      return;
+    }
+    this.setState({ selectedCamera: data.value });
+  };
+
   componentDidMount() {
-    Events.on('cameratoggle', (data) => {
-      if (this.justChangedCamera) {
-        // Prevent recursion.
-        this.justChangedCamera = false;
-        return;
-      }
-      this.setState({ selectedCamera: data.value });
-    });
+    Events.on('cameratoggle', this.onCameraToggle);
+  }
+
+  componentWillUnmount() {
+    Events.off('cameratoggle', this.onCameraToggle);
   }
 
   onChange(option) {

diff --git a/src/components/viewport/TransformToolbar.js b/src/components/viewport/TransformToolbar.js
@@ -26,18 +26,26 @@ export default class TransformToolbar extends React.Component {
     };
   }
 
+  onTransformModeChange = (mode) => {
+    this.setState({ selectedTransform: mode });
+  };
+
+  onTransformSpaceChange = () => {
+    Events.emit(
+      'transformspacechanged',
+      this.state.localSpace ? 'world' : 'local'
+    );
+    this.setState({ localSpace: !this.state.localSpace });
+  };
+
   componentDidMount() {
-    Events.on('transformmodechange', (mode) => {
-      this.setState({ selectedTransform: mode });
-    });
+    Events.on('transformmodechange', this.onTransformModeChange);
+    Events.on('transformspacechange', this.onTransformSpaceChange);
+  }
 
-    Events.on('transformspacechange', () => {
-      Events.emit(
-        'transformspacechanged',
-        this.state.localSpace ? 'world' : 'local'
-      );
-      this.setState({ localSpace: !this.state.localSpace });
-    });
+  componentWillUnmount() {
+    Events.off('transformmodechange', this.onTransformModeChange);
+    Events.off('transformspacechange', this.onTransformSpaceChange);
   }
 
   changeTransformMode = (mode) => {

diff --git a/src/components/viewport/ViewportHUD.js b/src/components/viewport/ViewportHUD.js
@@ -11,14 +11,22 @@ export default class ViewportHUD extends React.Component {
     };
   }
 
+  onRaycasterMouseEnter = (el) => {
+    this.setState({ hoveredEntity: el });
+  };
+
+  onRaycasterMouseLeave = (el) => {
+    this.setState({ hoveredEntity: el });
+  };
+
   componentDidMount() {
-    Events.on('raycastermouseenter', (el) => {
-      this.setState({ hoveredEntity: el });
-    });
+    Events.on('raycastermouseenter', this.onRaycasterMouseEnter);
+    Events.on('raycastermouseleave', this.onRaycasterMouseLeave);
+  }
 
-    Events.on('raycastermouseleave', (el) => {
-      this.setState({ hoveredEntity: el });
-    });
+  componentWillUnmount() {
+    Events.off('raycastermouseenter', this.onRaycasterMouseEnter);
+    Events.off('raycastermouseleave', this.onRaycasterMouseLeave);
   }
 
   render() {