fix(react-components): Ensure that SceneContainer only registers "all…

…-loaded" once (#4932) * fix(react-components): Ensure that SceneContainer only registers "all-loaded" once * chore: lint fix * chore: use input resources as ground thruth of expected resources * chore: make sure loaderror counts reset * chore: move functions, lint fix * chore: don't use useEffect for cleanup * chore: lint fix
cognitedata · Dec 4, 2024 · cf731a8 · cf731a8
1 parent 923b974
commit cf731a8
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 90 deletions.
diff --git a/react-components/src/components/CadModelContainer/CadModelContainer.tsx b/react-components/src/components/CadModelContainer/CadModelContainer.tsx
@@ -13,8 +13,7 @@ import { type Matrix4 } from 'three';
 import { useRevealKeepAlive } from '../RevealKeepAlive/RevealKeepAliveContext';
 import {
   useReveal3DResourceLoadFailCount,
-  useReveal3DResourcesCount,
-  useThisAsExpectedResourceLoad
+  useReveal3DResourcesCount
 } from '../Reveal3DResources/Reveal3DResourcesInfoContext';
 import { isEqual } from 'lodash';
 import { modelExists } from '../../utilities/modelExists';
@@ -47,7 +46,6 @@ export function CadModelContainer({
 
   const [model, setModel] = useState<CogniteCadModel | undefined>(undefined);
 
-  useThisAsExpectedResourceLoad();
   const { modelId, revisionId, geometryFilter } = addModelOptions;
 
   useEffect(() => {
@@ -56,10 +54,11 @@ export function CadModelContainer({
     }
 
     initializingModel.current = addModelOptions;
-    addModel(addModelOptions, transform)
+    const cleanupCallbackPromise = addModel(addModelOptions, transform)
       .then((model) => {
         onLoad?.(model);
         setRevealResourcesCount(getViewerResourceCount(viewer));
+        return removeModel;
       })
       .catch((error) => {
         const errorReportFunction = onLoadError ?? defaultLoadErrorHandler;
@@ -69,6 +68,12 @@ export function CadModelContainer({
           setReveal3DResourceLoadFailCount((p) => p - 1);
         };
       });
+
+    return () => {
+      void cleanupCallbackPromise.then((callback) => {
+        callback();
+      });
+    };
   }, [modelId, revisionId, geometryFilter]);
 
   useEffect(() => {
@@ -79,13 +84,6 @@ export function CadModelContainer({
 
   useApplyCadModelStyling(model, styling);
 
-  useEffect(
-    () => () => {
-      removeModel(model);
-    },
-    [model]
-  );
-
   return <></>;
 
   async function addModel(
@@ -116,7 +114,7 @@ export function CadModelContainer({
     }
   }
 
-  function removeModel(model: CogniteCadModel | undefined): void {
+  function removeModel(): void {
     if (!modelExists(model, viewer)) return;
 
     if (cachedViewerRef !== undefined && !cachedViewerRef.isRevealContainerMountedRef.current)

diff --git a/react-components/src/components/Image360CollectionContainer/Image360CollectionContainer.tsx b/react-components/src/components/Image360CollectionContainer/Image360CollectionContainer.tsx
@@ -17,8 +17,7 @@ import {
 } from './constants';
 import {
   useReveal3DResourceLoadFailCount,
-  useReveal3DResourcesCount,
-  useThisAsExpectedResourceLoad
+  useReveal3DResourcesCount
 } from '../Reveal3DResources/Reveal3DResourcesInfoContext';
 import { getViewerResourceCount } from '../../utilities/getViewerResourceCount';
 
@@ -45,8 +44,6 @@ export function Image360CollectionContainer({
     undefined
   );
 
-  useThisAsExpectedResourceLoad();
-
   useEffect(() => {
     if (
       addImage360CollectionOptions.source === 'events' &&
@@ -57,8 +54,12 @@ export function Image360CollectionContainer({
 
     initializingSiteId.current = addImage360CollectionOptions;
 
-    void add360Collection(addImage360CollectionOptions.transform);
-    return remove360Collection;
+    const cleanupCallbackPromise = add360Collection(addImage360CollectionOptions.transform);
+    return () => {
+      void cleanupCallbackPromise.then((callback) => {
+        callback();
+      });
+    };
   }, [addImage360CollectionOptions]);
 
   useApply360AnnotationStyling(modelRef.current, styling);
@@ -78,8 +79,8 @@ export function Image360CollectionContainer({
 
   return <></>;
 
-  async function add360Collection(transform?: Matrix4): Promise<void> {
-    await getOrAdd360Collection()
+  async function add360Collection(transform?: Matrix4): Promise<() => void> {
+    return await getOrAdd360Collection()
       .then((image360Collection) => {
         if (transform !== undefined) {
           image360Collection.setModelTransformation(transform);
@@ -93,6 +94,7 @@ export function Image360CollectionContainer({
         modelRef.current = image360Collection;
         onLoad?.(image360Collection);
         setRevealResourcesCount(getViewerResourceCount(viewer));
+        return remove360Collection;
       })
       .catch((error: any) => {
         const errorReportFunction = onLoadError ?? defaultLoadErrorHandler;

diff --git a/react-components/src/components/PointCloudContainer/PointCloudContainer.tsx b/react-components/src/components/PointCloudContainer/PointCloudContainer.tsx
@@ -13,8 +13,7 @@ import { useReveal } from '../RevealCanvas/ViewerContext';
 import { useRevealKeepAlive } from '../RevealKeepAlive/RevealKeepAliveContext';
 import {
   useReveal3DResourceLoadFailCount,
-  useReveal3DResourcesCount,
-  useThisAsExpectedResourceLoad
+  useReveal3DResourcesCount
 } from '../Reveal3DResources/Reveal3DResourcesInfoContext';
 import { cloneDeep, isEqual } from 'lodash';
 import { useApplyPointCloudStyling } from './useApplyPointCloudStyling';
@@ -47,8 +46,6 @@ export function PointCloudContainer({
   const { setReveal3DResourceLoadFailCount } = useReveal3DResourceLoadFailCount();
   const initializingModel = useRef<AddModelOptions<DataSourceType> | undefined>(undefined);
 
-  useThisAsExpectedResourceLoad();
-
   const [{ data: addModelOptionsResult }] = useModelIdRevisionIdFromModelOptions([addModelOptions]);
 
   const modelId = addModelOptionsResult?.modelId;
@@ -65,10 +62,11 @@ export function PointCloudContainer({
 
     initializingModel.current = cloneDeep(addModelOptions);
 
-    addModel(addModelOptions, transform)
+    const cleanupCallbackPromise = addModel(addModelOptions, transform)
       .then((pointCloudModel: CognitePointCloudModel<DataSourceType>) => {
         onLoad?.(pointCloudModel);
         setRevealResourcesCount(getViewerResourceCount(viewer));
+        return removeModel;
       })
       .catch((error) => {
         const errorHandler = onLoadError ?? defaultLoadErrorHandler;
@@ -78,6 +76,12 @@ export function PointCloudContainer({
           setReveal3DResourceLoadFailCount((p) => p - 1);
         };
       });
+
+    return () => {
+      void cleanupCallbackPromise.then((callback) => {
+        callback();
+      });
+    };
   }, [modelId, revisionId]);
 
   useEffect(() => {
@@ -88,8 +92,6 @@ export function PointCloudContainer({
 
   useApplyPointCloudStyling(model, styling);
 
-  useEffect(() => removeModel, [model]);
-
   return <></>;
 
   async function addModel(

diff --git a/react-components/src/components/Reveal3DResources/Reveal3DResources.tsx b/react-components/src/components/Reveal3DResources/Reveal3DResources.tsx
@@ -1,22 +1,12 @@
 /*!
  * Copyright 2023 Cognite AS
  */
-import { useRef, type ReactElement, useEffect, useMemo } from 'react';
-import {
-  type DataSourceType,
-  type CogniteCadModel,
-  type CognitePointCloudModel,
-  type Image360Collection
-} from '@cognite/reveal';
+import { type ReactElement, useEffect, useMemo } from 'react';
 import { CadModelContainer } from '../CadModelContainer/CadModelContainer';
 import { PointCloudContainer } from '../PointCloudContainer/PointCloudContainer';
 import { Image360CollectionContainer } from '../Image360CollectionContainer/Image360CollectionContainer';
 import { useReveal } from '../RevealCanvas/ViewerContext';
-import {
-  type AddResourceOptions,
-  type Reveal3DResourcesProps,
-  type CadModelOptions
-} from './types';
+import { type Reveal3DResourcesProps, type CadModelOptions } from './types';
 import { useCalculatePointCloudStyling } from './hooks/useCalculatePointCloudStyling';
 import { EMPTY_ARRAY } from '../../utilities/constants';
 import {
@@ -37,6 +27,8 @@ import {
   useGenerateAssetMappingCachePerItemFromModelCache,
   useGenerateNode3DCache
 } from '../../hooks/cad';
+import { useCallCallbackOnFinishedLoading } from './hooks/useCallCallbackOnFinishedLoading';
+import { useSetExpectedLoadCount } from './hooks/useSetExpectedLoadCount';
 
 export const Reveal3DResources = ({
   resources,
@@ -48,12 +40,14 @@ export const Reveal3DResources = ({
   image360Settings
 }: Reveal3DResourcesProps): ReactElement => {
   const viewer = useReveal();
-  const numModelsLoaded = useRef(0);
 
   useRemoveNonReferencedModels(resources, viewer);
 
   const { data: reveal3DModels } = useTypedModels(viewer, resources, onResourceLoadError);
 
+  useSetExpectedLoadCount(resources);
+  useCallCallbackOnFinishedLoading(resources, onResourcesAdded);
+
   const image360CollectionAddOptions = useMemo(() => {
     return resources
       .filter(is360ImageAddOptions)
@@ -104,34 +98,6 @@ export const Reveal3DResources = ({
           group.style !== undefined
       ) ?? EMPTY_ARRAY;
 
-  const onModelLoaded = (
-    model:
-      | CogniteCadModel
-      | CognitePointCloudModel<DataSourceType>
-      | Image360Collection<DataSourceType>
-  ): void => {
-    onModelFailOrSucceed();
-    onResourceIsLoaded?.(model);
-  };
-
-  const onModelLoadedError = (addOptions: AddResourceOptions, error: any): void => {
-    onResourceLoadError?.(addOptions, error);
-    onModelFailOrSucceed();
-  };
-
-  const onModelFailOrSucceed = (): void => {
-    if (reveal3DModels === undefined) {
-      return;
-    }
-    numModelsLoaded.current += 1;
-
-    const expectedTotalLoadCount = reveal3DModels.length + image360CollectionAddOptions.length;
-
-    if (numModelsLoaded.current === expectedTotalLoadCount && onResourcesAdded !== undefined) {
-      onResourcesAdded();
-    }
-  };
-
   return (
     <>
       {styledCadModelOptions.map(({ styleGroups, model }, index) => {
@@ -146,7 +112,7 @@ export const Reveal3DResources = ({
             addModelOptions={model}
             styling={cadStyling}
             transform={model.transform}
-            onLoad={onModelLoaded}
+            onLoad={onResourceIsLoaded}
             onLoadError={onResourceLoadError}
           />
         );
@@ -169,8 +135,8 @@ export const Reveal3DResources = ({
             addModelOptions={model}
             styling={pcStyling}
             transform={model.transform}
-            onLoad={onModelLoaded}
-            onLoadError={onModelLoadedError}
+            onLoad={onResourceIsLoaded}
+            onLoadError={onResourceLoadError}
           />
         );
       })}
@@ -192,8 +158,8 @@ export const Reveal3DResources = ({
               key={key}
               addImage360CollectionOptions={addModelOption}
               styling={image360Styling}
-              onLoad={onModelLoaded}
-              onLoadError={onModelLoadedError}
+              onLoad={onResourceIsLoaded}
+              onLoadError={onResourceLoadError}
             />
           );
         }

diff --git a/react-components/src/components/Reveal3DResources/Reveal3DResourcesInfoContext.tsx b/react-components/src/components/Reveal3DResources/Reveal3DResourcesInfoContext.tsx
@@ -10,8 +10,7 @@ import {
   type ReactNode,
   useMemo,
   type Dispatch,
-  type SetStateAction,
-  useEffect
+  type SetStateAction
 } from 'react';
 
 type Reveal3DResourcesInfoContent = {
@@ -60,18 +59,13 @@ export const useReveal3DResourcesExpectedInViewerCount = (): number => {
   return element.reveal3DResourcesExpectedToLoad - element.reveal3DResourceLoadFailCount;
 };
 
-export const useReveal3DLoadedResourceCount = (): number => {
-  return useInfoElementOfContext().reveal3DResourcesCount;
+export const useReveal3DResourcesSetExpectedToLoadCount = (): Dispatch<SetStateAction<number>> => {
+  const element = useInfoElementOfContext();
+  return element.setReveal3DResourcesExpectedToLoad;
 };
 
-export const useThisAsExpectedResourceLoad = (): void => {
-  const element = useInfoElementOfContext();
-  useEffect(() => {
-    element.setReveal3DResourcesExpectedToLoad((prev) => prev + 1);
-    return () => {
-      element.setReveal3DResourcesExpectedToLoad((prev) => prev - 1);
-    };
-  }, []);
+export const useReveal3DLoadedResourceCount = (): number => {
+  return useInfoElementOfContext().reveal3DResourcesCount;
 };
 
 export const useReveal3DResourcesStylingLoading = (): boolean => {

diff --git a/react-components/src/components/Reveal3DResources/hooks/useCallCallbackOnFinishedLoading.ts b/react-components/src/components/Reveal3DResources/hooks/useCallCallbackOnFinishedLoading.ts
@@ -0,0 +1,25 @@
+/*!
+ * Copyright 2024 Cognite AS
+ */
+import { useEffect } from 'react';
+import { type AddResourceOptions } from '../types';
+import {
+  useReveal3DResourceLoadFailCount,
+  useReveal3DResourcesCount,
+  useReveal3DResourcesExpectedInViewerCount
+} from '../Reveal3DResourcesInfoContext';
+
+export function useCallCallbackOnFinishedLoading(
+  resources: AddResourceOptions[],
+  onResourcesAdded: (() => void) | undefined
+): void {
+  const loadedCount = useReveal3DResourcesCount().reveal3DResourcesCount;
+  const expectedLoadCount = useReveal3DResourcesExpectedInViewerCount();
+  const { reveal3DResourceLoadFailCount } = useReveal3DResourceLoadFailCount();
+
+  useEffect(() => {
+    if (loadedCount === resources.length - reveal3DResourceLoadFailCount) {
+      onResourcesAdded?.();
+    }
+  }, [loadedCount, expectedLoadCount]);
+}
diff --git a/react-components/src/components/Reveal3DResources/hooks/useSetExpectedLoadCount.ts b/react-components/src/components/Reveal3DResources/hooks/useSetExpectedLoadCount.ts
@@ -0,0 +1,18 @@
+/*!
+ * Copyright 2024 Cognite AS
+ */
+import { useEffect } from 'react';
+import { type AddResourceOptions } from '..';
+import {
+  useReveal3DResourcesCount,
+  useReveal3DResourcesSetExpectedToLoadCount
+} from '../Reveal3DResourcesInfoContext';
+
+export function useSetExpectedLoadCount(resources: AddResourceOptions[]): void {
+  const setExpectedToLoadCount = useReveal3DResourcesSetExpectedToLoadCount();
+  const { setRevealResourcesCount } = useReveal3DResourcesCount();
+
+  useEffect(() => {
+    setExpectedToLoadCount(resources.length);
+  }, [resources, setRevealResourcesCount]);
+}