fix: plugin API update event gets called repeatedly, missing hook deps (

#306) * add error boundary * fix infinite loop * update yarn.lock * update eslint and fix hook deps * Prevented a slight change in the width of the right panel when selecting layers * fix eslint config
reearth · Sep 2, 2022 · 47ec244 · 47ec244
1 parent f580e6d
commit 47ec244
Show file tree

Hide file tree

Showing 18 changed files with 396 additions and 263 deletions.
diff --git a/.eslintrc.yml b/.eslintrc.yml
@@ -14,6 +14,10 @@ rules:
         order: asc
         caseInsensitive: true
 overrides:
+  - files:
+      - i18next-parser.config.js
+    extends:
+      - reearth/commonjs
   - files:
       - src/gql/queries/*.ts
       - src/gql/fragments/*.ts

diff --git a/package.json b/package.json
@@ -66,16 +66,16 @@
         "cypress": "10.3.1",
         "del-cli": "4.0.1",
         "dotenv": "16.0.1",
-        "eslint": "8.16.0",
-        "eslint-config-reearth": "0.1.0",
+        "eslint": "8.23.0",
+        "eslint-config-reearth": "0.1.1",
         "eslint-plugin-cypress": "2.12.1",
         "eslint-plugin-graphql": "4.0.0",
         "husky": "8.0.1",
         "i18next-parser": "6.4.0",
         "jsdom": "20.0.0",
         "lint-staged": "12.4.2",
         "npm-run-all": "4.1.5",
-        "prettier": "2.6.2",
+        "prettier": "2.7.1",
         "read-env": "2.0.0",
         "rollup-plugin-visualizer": "5.7.1",
         "ts-node": "10.8.1",
@@ -85,7 +85,7 @@
         "vite": "3.0.3",
         "vite-plugin-cesium": "1.2.22",
         "vitest": "0.19.0",
-        "web-streams-polyfill": "^3.2.1"
+        "web-streams-polyfill": "3.2.1"
     },
     "dependencies": {
         "@apollo/client": "3.6.6",
@@ -135,6 +135,7 @@
         "react-dnd-html5-backend": "16.0.1",
         "react-dom": "18.1.0",
         "react-dropzone": "14.2.1",
+        "react-error-boundary": "3.1.4",
         "react-ga": "3.3.0",
         "react-i18next": "11.17.1",
         "react-inlinesvg": "3.0.0",

diff --git a/src/components/atoms/Plugin/PluginIFrame/useIFrame.ts b/src/components/atoms/Plugin/PluginIFrame/useIFrame.ts
@@ -42,7 +42,7 @@ export default function useIFrame({
       },
       postMessage,
     }),
-    [],
+    [iframeCanBeVisible, onRender, postMessage],
   );
 
   useEffect(() => {

diff --git a/src/components/atoms/Plugin/hooks.ts b/src/components/atoms/Plugin/hooks.ts
@@ -80,15 +80,24 @@ export default function useHook({
 
   const messageEvents = useMemo(() => new Set<(msg: any) => void>(), []);
   const messageOnceEvents = useMemo(() => new Set<(msg: any) => void>(), []);
-  const onMessage = useCallback((e: (msg: any) => void) => {
-    messageEvents.add(e);
-  }, []);
-  const offMessage = useCallback((e: (msg: any) => void) => {
-    messageEvents.delete(e);
-  }, []);
-  const onceMessage = useCallback((e: (msg: any) => void) => {
-    messageOnceEvents.add(e);
-  }, []);
+  const onMessage = useCallback(
+    (e: (msg: any) => void) => {
+      messageEvents.add(e);
+    },
+    [messageEvents],
+  );
+  const offMessage = useCallback(
+    (e: (msg: any) => void) => {
+      messageEvents.delete(e);
+    },
+    [messageEvents],
+  );
+  const onceMessage = useCallback(
+    (e: (msg: any) => void) => {
+      messageOnceEvents.add(e);
+    },
+    [messageOnceEvents],
+  );
   const handleMessage = useCallback(
     (msg: any) => {
       try {
@@ -100,7 +109,7 @@ export default function useHook({
       rawOnMessage?.(msg);
       messageOnceEvents.clear();
     },
-    [messageEvents],
+    [messageEvents, messageOnceEvents, onError, rawOnMessage],
   );
 
   const evalCode = useCallback(
@@ -178,11 +187,13 @@ export default function useHook({
       setLoaded(true);
     })();
 
+    const iframeRef = mainIFrameRef.current;
+
     return () => {
       onDispose?.();
       messageEvents.clear();
       messageOnceEvents.clear();
-      mainIFrameRef.current?.reset();
+      iframeRef?.reset();
       setLoaded(false);
       if (typeof eventLoop.current === "number") {
         window.clearTimeout(eventLoop.current);
@@ -198,7 +209,20 @@ export default function useHook({
         }
       }
     };
-  }, [code, evalCode, isMarshalable, onDispose, onPreInit, skip, exposed]);
+  }, [
+    code,
+    evalCode,
+    isMarshalable,
+    onDispose,
+    onPreInit,
+    skip,
+    exposed,
+    onMessage,
+    offMessage,
+    onceMessage,
+    messageEvents,
+    messageOnceEvents,
+  ]);
 
   useImperativeHandle(
     ref,

diff --git a/src/components/atoms/Resizable/index.tsx b/src/components/atoms/Resizable/index.tsx
@@ -50,6 +50,7 @@ const StyledResizable = styled.div<Pick<Props, "direction" | "size">>`
   flex-direction: ${({ direction }) => (direction === "vertical" ? "row" : "column")};
   width: ${({ direction, size }) => (direction === "horizontal" ? null : `${size}px`)};
   height: ${({ direction, size }) => (direction === "vertical" ? null : `${size}px`)};
+  flex-shrink: 0;
 `;
 
 const Wrapper = styled.div`

diff --git a/src/components/atoms/Timeline/hooks.ts b/src/components/atoms/Timeline/hooks.ts
@@ -139,17 +139,20 @@ const useTimelinePlayer = ({
   const [isPlaying, setIsPlaying] = useState(false);
   const [isPlayingReversed, setIsPlayingReversed] = useState(false);
   const syncCurrentTimeRef = useRef(currentTime);
-  const handleOnSpeedChange: ChangeEventHandler<HTMLInputElement> = useCallback(e => {
-    onSpeedChange?.(parseInt(e.currentTarget.value, 10));
-  }, []);
+  const handleOnSpeedChange: ChangeEventHandler<HTMLInputElement> = useCallback(
+    e => {
+      onSpeedChange?.(parseInt(e.currentTarget.value, 10));
+    },
+    [onSpeedChange],
+  );
   const toggleIsPlaying = useCallback(() => {
     if (isPlayingReversed) {
       setIsPlayingReversed(false);
       onPlayReversed?.(false);
     }
     setIsPlaying(p => !p);
     onPlay?.(!isPlaying);
-  }, [isPlayingReversed, isPlaying, onPlay, onPlayReversed]);
+  }, [isPlayingReversed, onPlay, isPlaying, onPlayReversed]);
   const toggleIsPlayingReversed = useCallback(() => {
     if (isPlaying) {
       setIsPlaying(false);

diff --git a/src/components/molecules/Visualizer/Engine/Cesium/cameraLimiter.ts b/src/components/molecules/Visualizer/Engine/Cesium/cameraLimiter.ts
@@ -26,19 +26,29 @@ export function useCameraLimiter(
   camera: Camera | undefined,
   property: SceneProperty["cameraLimiter"] | undefined,
 ) {
-  const geodesic = useMemo(():
-    | undefined
-    | { geodesicVertical: EllipsoidGeodesic; geodesicHorizontal: EllipsoidGeodesic } => {
+  const geodesic = useMemo(() => {
     const viewer = cesium.current?.cesiumElement;
-    if (!viewer || viewer.isDestroyed()) return undefined;
-    return property?.cameraLimitterTargetArea?.lng && property.cameraLimitterTargetArea.lat
-      ? getGeodesic(
-          viewer,
-          property.cameraLimitterTargetArea.lng,
-          property.cameraLimitterTargetArea.lat,
-        )
-      : undefined;
-  }, [cesium, property?.cameraLimitterTargetArea?.lng, property?.cameraLimitterTargetArea?.lat]);
+    if (
+      !viewer ||
+      viewer.isDestroyed() ||
+      !property?.cameraLimitterEnabled ||
+      !property.cameraLimitterTargetArea?.lng ||
+      !property.cameraLimitterTargetArea.lat
+    ) {
+      return undefined;
+    }
+
+    return getGeodesic(
+      viewer,
+      property.cameraLimitterTargetArea.lng,
+      property.cameraLimitterTargetArea.lat,
+    );
+  }, [
+    cesium,
+    property?.cameraLimitterEnabled,
+    property?.cameraLimitterTargetArea?.lng,
+    property?.cameraLimitterTargetArea?.lat,
+  ]);
 
   // calculate inner limiter dimensions
   const limiterDimensions = useMemo((): InnerLimiterDimensions | undefined => {
@@ -132,7 +142,7 @@ export function useCameraLimiter(
   }, [camera, cesium, property, limiterDimensions]);
 
   return {
-    limiterDimensions,
+    cameraViewBoundaries: limiterDimensions?.cartesianArray,
     cameraViewOuterBoundaries,
     cameraViewBoundariesMaterial,
   };
@@ -156,7 +166,7 @@ export const getGeodesic = (
   viewer: CesiumViewer,
   lng: number,
   lat: number,
-): { geodesicVertical: EllipsoidGeodesic; geodesicHorizontal: EllipsoidGeodesic } | undefined => {
+): { vertical: EllipsoidGeodesic; horizontal: EllipsoidGeodesic } | undefined => {
   const ellipsoid = viewer.scene.globe.ellipsoid;
 
   const centerPoint = Cartesian3.fromDegrees(lng, lat, 0);
@@ -172,21 +182,21 @@ export const getGeodesic = (
     new Cartesian3(),
   );
 
-  const geodesicVertical = new EllipsoidGeodesic(
+  const vertical = new EllipsoidGeodesic(
     cartographicCenterPoint,
     Cartographic.fromCartesian(north),
     ellipsoid,
   );
-  const geodesicHorizontal = new EllipsoidGeodesic(
+  const horizontal = new EllipsoidGeodesic(
     cartographicCenterPoint,
     Cartographic.fromCartesian(east),
     ellipsoid,
   );
-  return { geodesicVertical, geodesicHorizontal };
+  return { vertical, horizontal };
 };
 
 export const calcBoundaryBox = (
-  geodesic: { geodesicVertical: EllipsoidGeodesic; geodesicHorizontal: EllipsoidGeodesic },
+  geodesic: { vertical: EllipsoidGeodesic; horizontal: EllipsoidGeodesic },
   halfLength: number,
   halfWidth: number,
 ): {
@@ -198,10 +208,10 @@ export const calcBoundaryBox = (
   };
   cartesianArray: Cartesian3[];
 } => {
-  const topDimension = geodesic.geodesicVertical.interpolateUsingSurfaceDistance(halfLength);
-  const bottomDimension = geodesic.geodesicVertical.interpolateUsingSurfaceDistance(-halfLength);
-  const rightDimension = geodesic.geodesicHorizontal.interpolateUsingSurfaceDistance(halfWidth);
-  const leftDimension = geodesic.geodesicHorizontal.interpolateUsingSurfaceDistance(-halfWidth);
+  const topDimension = geodesic.vertical.interpolateUsingSurfaceDistance(halfLength);
+  const bottomDimension = geodesic.vertical.interpolateUsingSurfaceDistance(-halfLength);
+  const rightDimension = geodesic.horizontal.interpolateUsingSurfaceDistance(halfWidth);
+  const leftDimension = geodesic.horizontal.interpolateUsingSurfaceDistance(-halfWidth);
 
   const rightTop = new Cartographic(rightDimension.longitude, topDimension.latitude, 0);
   const leftTop = new Cartographic(leftDimension.longitude, topDimension.latitude, 0);

diff --git a/src/components/molecules/Visualizer/Engine/Cesium/hooks.ts b/src/components/molecules/Visualizer/Engine/Cesium/hooks.ts
@@ -350,15 +350,15 @@ export default ({
       cesiumDnD.current?.disable();
     };
   }, [handleLayerDrag, handleLayerDrop, isLayerDraggable]);
-  const { limiterDimensions, cameraViewOuterBoundaries, cameraViewBoundariesMaterial } =
+  const { cameraViewBoundaries, cameraViewOuterBoundaries, cameraViewBoundariesMaterial } =
     useCameraLimiter(cesium, camera, property?.cameraLimiter);
 
   return {
     terrainProvider,
     terrainProperty,
     backgroundColor,
     cesium,
-    limiterDimensions,
+    cameraViewBoundaries,
     cameraViewOuterBoundaries,
     cameraViewBoundariesMaterial,
     handleMount,

diff --git a/src/components/molecules/Visualizer/Engine/Cesium/index.tsx b/src/components/molecules/Visualizer/Engine/Cesium/index.tsx
@@ -53,7 +53,7 @@ const Cesium: React.ForwardRefRenderFunction<EngineRef, EngineProps> = (
     terrainProperty,
     backgroundColor,
     cesium,
-    limiterDimensions,
+    cameraViewBoundaries,
     cameraViewOuterBoundaries,
     cameraViewBoundariesMaterial,
     handleMount,
@@ -145,10 +145,10 @@ const Cesium: React.ForwardRefRenderFunction<EngineRef, EngineProps> = (
           percentageChanged={0.2}
           onMoveEnd={handleCameraMoveEnd}
         />
-        {limiterDimensions && property?.cameraLimiter?.cameraLimitterShowHelper && (
+        {cameraViewBoundaries && property?.cameraLimiter?.cameraLimitterShowHelper && (
           <Entity>
             <PolylineGraphics
-              positions={limiterDimensions.cartesianArray}
+              positions={cameraViewBoundaries}
               width={1}
               material={Color.RED}
               arcType={ArcType.RHUMB}></PolylineGraphics>

diff --git a/src/components/molecules/Visualizer/Error/index.tsx b/src/components/molecules/Visualizer/Error/index.tsx
@@ -0,0 +1,15 @@
+import type { FallbackProps } from "react-error-boundary";
+
+export default function Error({ error, resetErrorBoundary }: FallbackProps) {
+  return (
+    <div>
+      <h1>Oops! An Error Occurred</h1>
+      <p>{error.message}</p>
+      <p>
+        <button style={{ color: "#fff" }} onClick={resetErrorBoundary}>
+          Retry
+        </button>
+      </p>
+    </div>
+  );
+}
diff --git a/src/components/molecules/Visualizer/Widget/Timeline/hooks.ts b/src/components/molecules/Visualizer/Widget/Timeline/hooks.ts
@@ -93,7 +93,7 @@ export const useTimeline = () => {
         clock.tick();
       });
     }
-  }, []);
+  }, [clock]);
 
   // Sync cesium clock.
   useEffect(() => {
@@ -106,7 +106,14 @@ export const useTimeline = () => {
       return prev;
     });
     setSpeed(Math.abs(clockSpeed));
-  }, [clockCurrentTime, clockStartTime, clockStopTime, clockSpeed]);
+  }, [
+    clockCurrentTime,
+    clockStartTime,
+    clockStopTime,
+    clockSpeed,
+    clock?.startTime,
+    clock?.stopTime,
+  ]);
 
   return {
     speed,

diff --git a/src/components/molecules/Visualizer/hooks.ts b/src/components/molecules/Visualizer/hooks.ts
@@ -72,9 +72,7 @@ export default ({
   onLayerDrop?: (layer: Layer, key: string, latlng: LatLng) => void;
 }) => {
   const engineRef = useRef<EngineRef>(null);
-  const [overriddenSceneProperty, overrideSceneProperty] = useOverriddenProperty(
-    sceneProperty ?? {},
-  );
+  const [overriddenSceneProperty, overrideSceneProperty] = useOverriddenProperty(sceneProperty);
 
   const wrapperRef = useRef<HTMLDivElement>(null);
   const { ref: dropRef, isDroppable } = useDrop(