Merge pull request #1368 from silx-kit/fix-usecamerastate

Fix regression in `useCameraState` when dependencies change

apps/storybook/src/Annotation.stories.tsx

@@ -1,14 +1,16 @@
-import { Annotation, DefaultInteractions, VisCanvas } from '@h5web/lib';
+import type { CanvasEvent } from '@h5web/lib';
+import { Annotation, useCanvasEvents } from '@h5web/lib';
+import { useRafState } from '@react-hookz/web';
 import type { Meta, Story } from '@storybook/react';
+import { useCallback } from 'react';
+import type { Vector3 } from 'three';
 
+import DefaultCanvas from './decorators/DefaultCanvas';
 import FillHeight from './decorators/FillHeight';
+import { formatCoord } from './utils';
 
 export const Default: Story = () => (
-  <VisCanvas
-    abscissaConfig={{ visDomain: [0, 41], showGrid: true }}
-    ordinateConfig={{ visDomain: [0, 20], showGrid: true }}
-  >
-    <DefaultInteractions />
+  <>
     <Annotation x={10} y={16}>
       HTML annotation positioned at (10, 16)
     </Annotation>
@@ -67,15 +69,11 @@ export const Default: Story = () => (
         </svg>
       </>
     </Annotation>
-  </VisCanvas>
+  </>
 );
 
 export const WithZoom: Story = () => (
-  <VisCanvas
-    abscissaConfig={{ visDomain: [0, 41], showGrid: true }}
-    ordinateConfig={{ visDomain: [0, 20], showGrid: true }}
-  >
-    <DefaultInteractions />
+  <>
     <Annotation x={10} y={16} scaleOnZoom style={{ width: 230 }}>
       HTML annotation at (10, 16) that scales with zoom.
     </Annotation>
@@ -89,11 +87,37 @@ export const WithZoom: Story = () => (
       Another annotation that scales with zoom but this time{' '}
       <strong>centred</strong> on (25, 10)
     </Annotation>
-  </VisCanvas>
+  </>
 );
 
+export const FollowPointer: Story = () => {
+  const [coords, setCoords] = useRafState<Vector3>();
+
+  const onPointerMove = useCallback(
+    (evt: CanvasEvent<PointerEvent>) => {
+      setCoords(evt.dataPt);
+    },
+    [setCoords]
+  );
+
+  useCanvasEvents({ onPointerMove });
+
+  if (!coords) {
+    return <></>; // eslint-disable-line react/jsx-no-useless-fragment
+  }
+
+  const { x, y } = coords;
+  return (
+    <Annotation
+      x={x + 0.5} // slight offset from pointer
+      y={y - 0.5}
+      style={{ whiteSpace: 'nowrap' }}
+    >{`x=${formatCoord(x)}, y=${formatCoord(y)}`}</Annotation>
+  );
+};
+
 export default {
   title: 'Building Blocks/Annotation',
   parameters: { layout: 'fullscreen' },
-  decorators: [FillHeight],
+  decorators: [DefaultCanvas, FillHeight],
 } as Meta;

apps/storybook/src/Utilities.stories.mdx

@@ -203,7 +203,7 @@ useCanvasEvents({ onPointerDown: handlePointerDown }); // also supported: `onPoi
 
 #### useCameraState
 
-Compute and update a local React state on every React Three Fiber frame. Useful to re-render React components when the user pans/zooms.
+Compute and update a state on every React Three Fiber frame. Useful to re-render React components when the user pans/zooms.
 The hook accepts a callback, which:
 
 - receives the `camera` object;
@@ -216,11 +216,22 @@ useCameraState<T>(
 ): T
 
 const { dataToHtml } = useVisCanvasContext();
-const htmlPt = useCameraState((camera) => dataToHtml(camera, new Vector3(x, y)));
+const htmlPt = useCameraState(
+  (camera) => dataToHtml(camera, new Vector3(x, y)),
+  [x, y, dataToHtml]
+);
+```
+
+`useCameraState` accepts a dependency array as second parameter (similarly to `useCallback`) and recomputes the state synchronously whenever any dependency changes. Make sure to configure ESLint's
+[`react-hooks/exhaustive-deps` rule](https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks#advanced-configuration)
+in your app so it can report missing dependencies:
+
+```json
+"react-hooks/exhaustive-deps": ["error", { "additionalHooks": "(useCameraState)" }]
 ```
 
-Internally, `useCameraState` uses React's `useState` and R3F's [`useFrame` hooks](https://docs.pmnd.rs/react-three-fiber/api/hooks#useframe).
-By default, React performs a strict equality check to avoid re-rendering the same state twice in a row. If your `factory` function returns a new object reference on every call,
+Internally, `useCameraState` uses R3F's [`useFrame` hooks](https://docs.pmnd.rs/react-three-fiber/api/hooks#useframe).
+By default, it performs a strict equality check with [`Object.is()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is) to avoid re-rendering the same state twice in a row. If your `factory` function returns a new object reference on every call,
 you can avoid unnecessary re-renders by providing your own equality function. Note, however, that this optimisation is pointless if `factory` always returns a different state when the camera changes. Example:
 
 ```ts
@@ -229,6 +240,7 @@ you can avoid unnecessary re-renders by providing your own equality function. No
  * the previous and next `pt` have the same coordinates. */
 const pt = useCameraState(
   (camera) => camera.scale > 0.5 ? camera.position.clone() : new Vector3()),
+  [], // no dependencies
   (prevPt, nextPt) => prevPt.equals(nextPt)
 );
 ```

apps/storybook/src/decorators/DefaultCanvas.tsx

@@ -0,0 +1,16 @@
+import { DefaultInteractions, VisCanvas } from '@h5web/lib';
+import type { Story } from '@storybook/react';
+
+function DefaultCanvas(MyStory: Story) {
+  return (
+    <VisCanvas
+      abscissaConfig={{ visDomain: [0, 41], showGrid: true }}
+      ordinateConfig={{ visDomain: [0, 20], showGrid: true }}
+    >
+      <DefaultInteractions />
+      <MyStory />
+    </VisCanvas>
+  );
+}
+
+export default DefaultCanvas;

apps/storybook/src/utils.ts

@@ -1,7 +1,7 @@
 import type { Rect } from '@h5web/lib';
 import { format } from 'd3-format';
 
-const formatCoord = format('.2f');
+export const formatCoord = format('.2f');
 
 export function getTitleForSelection(selection: Rect | undefined) {
   if (!selection) {

eslint.shared.js

@@ -48,6 +48,13 @@ module.exports = {
           rules: {
             'react/jsx-no-constructed-context-values': 'off', // too strict
             'react/no-unknown-property': 'off', // false positives with R3F
+
+            // `useCameraState` accepts an array of deps like `useEffect`
+            // https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks#advanced-configuration
+            'react-hooks/exhaustive-deps': [
+              'error',
+              { additionalHooks: '(useCameraState)' },
+            ],
           },
         }),
         createTypeScriptOverride({

packages/lib/src/toolbar/floating/ResetZoomButton.tsx

@@ -8,7 +8,10 @@ function ResetZoomButton() {
   const camera = useThree((state) => state.camera);
   const invalidate = useThree((state) => state.invalidate);
 
-  const isZoomedIn = useCameraState(({ scale }) => scale.x < 1 || scale.y < 1);
+  const isZoomedIn = useCameraState(
+    ({ scale }) => scale.x < 1 || scale.y < 1,
+    []
+  );
 
   function resetZoom() {
     camera.scale.x = 1;

packages/lib/src/vis/hooks.ts

@@ -5,10 +5,10 @@ import {
   createMemo,
 } from '@h5web/shared';
 import type { Domain, AnyNumArray } from '@h5web/shared';
-import { useSyncedRef } from '@react-hookz/web';
+import { useRerender, useSyncedRef } from '@react-hookz/web';
 import type { Camera } from '@react-three/fiber';
 import { useFrame, useThree } from '@react-three/fiber';
-import { useCallback, useMemo, useState } from 'react';
+import { useCallback, useMemo, useRef, useState } from 'react';
 import type { RefCallback } from 'react';
 
 import {
@@ -53,21 +53,34 @@ export function useDomains(
 
 export function useCameraState<T>(
   factory: (camera: Camera) => T,
-  equalityFn?: (prev: T, next: T) => boolean
+  deps: unknown[],
+  equalityFn = (prev: T, next: T) => Object.is(prev, next)
 ): T {
   const camera = useThree((state) => state.camera);
+  const rerender = useRerender();
 
-  const factoryRef = useSyncedRef(factory);
-  const [state, setState] = useState(() => factoryRef.current(camera));
+  const stateRef = useRef<T>(); // ref instead of state to avoid re-render when deps change
+  const factoryRef = useSyncedRef(factory); // ensure `useMemo` always sees latest `factory` reference
+
+  useMemo(() => {
+    // Compute state synchronously when deps change
+    stateRef.current = factoryRef.current(camera);
+  }, deps); // eslint-disable-line react-hooks/exhaustive-deps
 
   useFrame(() => {
+    // Recompute state and re-render on every frame
     const next = factoryRef.current(camera);
-    if (!equalityFn || !equalityFn(state, next)) {
-      setState(next);
+
+    // ... unless state hasn't changed
+    if (equalityFn(stateRef.current as T, next)) {
+      return;
     }
+
+    stateRef.current = next;
+    rerender();
   });
 
-  return state;
+  return stateRef.current as T; // synchronous update in `useMemo` guarantees `T` (which can include `undefined`)
 }
 
 export function useCssColors(

packages/lib/src/vis/shared/Annotation.tsx

@@ -32,10 +32,13 @@ function Annotation(props: Props) {
   }
 
   const { dataToHtml } = useVisCanvasContext();
-  const { htmlPt, cameraScale } = useCameraState((camera) => ({
-    htmlPt: dataToHtml(camera, new Vector3(x, y)),
-    cameraScale: camera.scale.clone(),
-  }));
+  const { htmlPt, cameraScale } = useCameraState(
+    (camera) => ({
+      htmlPt: dataToHtml(camera, new Vector3(x, y)),
+      cameraScale: camera.scale.clone(),
+    }),
+    [x, y, dataToHtml]
+  );
 
   const transforms = [
     center ? 'translate(-50%, -50%)' : '',

packages/lib/src/vis/shared/AxisSystem.tsx

@@ -16,7 +16,7 @@ function AxisSystem(props: Props) {
   const { canvasSize, abscissaConfig, ordinateConfig, getVisibleDomains } =
     useVisCanvasContext();
 
-  const { xVisibleDomain, yVisibleDomain } = useCameraState(getVisibleDomains);
+  const visibleDomains = useCameraState(getVisibleDomains, [getVisibleDomains]);
 
   return (
     // Append to `canvasWrapper` instead of `r3fRoot`
@@ -32,15 +32,15 @@ function AxisSystem(props: Props) {
         <Axis
           type="abscissa"
           config={abscissaConfig}
-          domain={xVisibleDomain}
+          domain={visibleDomains.xVisibleDomain}
           canvasSize={canvasSize}
           offset={axisOffsets.bottom}
           showAxis={showAxes}
         />
         <Axis
           type="ordinate"
           config={ordinateConfig}
-          domain={yVisibleDomain}
+          domain={visibleDomains.yVisibleDomain}
           canvasSize={canvasSize}
           offset={axisOffsets.left}
           showAxis={showAxes}

packages/lib/src/vis/shared/DataToHtml.tsx

@@ -14,9 +14,10 @@ function DataToHtml<T extends Vector3[]>(props: Props<T>) {
   const { points, children } = props;
 
   const { dataToHtml } = useVisCanvasContext();
-  const htmlPoints = useCameraState((camera) => {
-    return points.map((pt) => dataToHtml(camera, pt)) as MappedTuple<T>;
-  });
+  const htmlPoints = useCameraState(
+    (camera) => points.map((pt) => dataToHtml(camera, pt)) as MappedTuple<T>,
+    [points, dataToHtml]
+  );
 
   return <>{children(...htmlPoints)}</>;
 }

packages/lib/src/vis/tiles/TiledHeatmapMesh.tsx

@@ -1,6 +1,6 @@
 import { clamp, range } from 'lodash';
 import { useRef } from 'react';
-import type { Object3D } from 'three';
+import type { Group } from 'three';
 
 import { getInterpolator } from '../heatmap/utils';
 import { useCameraState } from '../hooks';
@@ -37,11 +37,11 @@ function TiledHeatmapMesh(props: Props) {
   const { canvasSize, visSize } = useVisCanvasContext();
   const meshSize = size ?? visSize;
 
-  const groupRef = useRef<Object3D>(null);
+  const groupRef = useRef<Group>(null);
 
   const ndcToLocalMatrix = useCameraState((camera) => {
     return getNdcToObject3DMatrix(camera, groupRef);
-  });
+  }, []);
   const visibleBox = getObject3DVisibleBox(ndcToLocalMatrix);
 
   const bounds = scaleBoxToLayer(visibleBox, baseLayerSize, meshSize);