chore: Exhaustive deps warning -> error (#13124)

* chore: Exhaustive deps turned to error

* Refactor useDragResize

* Refactor Lightbox to explicit state machine

* feedback

* ci

.oxlintrc.json

@@ -121,7 +121,8 @@
             "ignoreRestSiblings": true
           }
         ],
-        "react/rules-of-hooks": "error"
+        "react/rules-of-hooks": "error",
+        "react-hooks/exhaustive-deps": "error"
       },
       "plugins": ["eslint", "oxc", "react", "typescript", "import"]
     },

app/components/Collection/CollectionForm.tsx

@@ -54,6 +54,9 @@ const useIconColor = (collection?: Collection) => {
       (hasMultipleCollections && collectionColors.length === 1
         ? collectionColors[0]
         : randomElement(colorPalette)),
+    // Deliberately only keyed on the collection color so the randomly picked
+    // fallback stays stable while the form is open.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
     [collection?.color]
   );
   return iconColor;

app/components/DocumentViews.tsx

@@ -8,6 +8,7 @@ import type User from "~/models/User";
 import { Avatar, AvatarSize } from "~/components/Avatar";
 import ListItem from "~/components/List/Item";
 import PaginatedList from "~/components/PaginatedList";
+import { useComputed } from "~/hooks/useComputed";
 import useCurrentUser from "~/hooks/useCurrentUser";
 import useStores from "~/hooks/useStores";
 
@@ -20,23 +21,26 @@ function DocumentViews({ document }: Props) {
   const { views, presence } = useStores();
   const user = useCurrentUser();
   const locale = dateLocale(user.language);
-  const documentPresence = presence.get(document.id);
+  // Use Set for O(1) lookups, computed so the identity is only replaced when
-  const documentPresenceArray = documentPresence
+  // the observable presence for the document actually changes.
-    ? Array.from(documentPresence.values())
+  const presentIds = useComputed(() => {
-    : [];
+    const documentPresence = presence.get(document.id);
-
+    return new Set(
-  // Use Set for O(1) lookups and stable references
+      documentPresence
-  const presentIds = useMemo(
+        ? Array.from(documentPresence.values()).map((p) => p.userId)
-    () => new Set(documentPresenceArray.map((p) => p.userId)),
+        : []
-    [documentPresenceArray]
+    );
-  );
+  }, [presence, document.id]);
-  const editingIds = useMemo(
+  const editingIds = useComputed(() => {
-    () =>
+    const documentPresence = presence.get(document.id);
-      new Set(
+    return new Set(
-        documentPresenceArray.filter((p) => p.isEditing).map((p) => p.userId)
+      documentPresence
-      ),
+        ? Array.from(documentPresence.values())
-    [documentPresenceArray]
+            .filter((p) => p.isEditing)
-  );
+            .map((p) => p.userId)
+        : []
+    );
+  }, [presence, document.id]);
 
   // ensure currently present via websocket are always ordered first
   const documentViews = useMemo(

app/components/Lightbox.tsx

@@ -15,6 +15,7 @@ import {
   useContext,
   useEffect,
   useMemo,
+  useReducer,
   useRef,
   useState,
 } from "react";
@@ -65,28 +66,13 @@ import { useDocumentContext } from "./DocumentContext";
 import LightboxComments from "~/scenes/Document/components/Comments/LightboxComments";
 import { PortalContext } from "./Portal";
 import useHideElement from "~/hooks/useHideElement";
-
+import type { Status } from "./LightboxState";
-export enum LightboxStatus {
+import {
-  READY_TO_OPEN,
+  ImageStatus,
-  OPENING,
+  LightboxStatus,
-  OPENED,
+  initialStatus,
-  READY_TO_CLOSE,
+  reducer,
-  CLOSING,
+} from "./LightboxState";
-  CLOSED,
-}
-
-export enum ImageStatus {
-  LOADING,
-  ERROR,
-  LOADED,
-  MIN_ZOOM,
-  MAX_ZOOM,
-  ZOOMED,
-}
-type Status = {
-  lightbox: LightboxStatus | null;
-  image: ImageStatus | null;
-};
 
 type Animation = {
   fadeIn?: { apply: () => Keyframes; duration: number };
@@ -98,6 +84,35 @@ type Animation = {
 
 const ANIMATION_DURATION = 0.3 * Second.ms;
 
+/**
+ * Converts an SVG data URL, as produced by the diagram and mermaid embeds, into
+ * a blob that can be downloaded.
+ *
+ * @param dataURL The data URL to convert.
+ * @returns The blob, or undefined if the URL is not an SVG data URL.
+ */
+const svgDataURLToBlob = (dataURL: string) => {
+  // Match the SVG data URL format (with or without charset)
+  const match = dataURL.match(
+    /^data:image\/svg\+xml(?:;charset=utf-8)?,(.*)$/i
+  );
+  if (!match) {
+    return;
+  }
+
+  const encodedSVGData = match[1];
+  const decodedSVGData = decodeURIComponent(encodedSVGData);
+
+  // Convert string to Uint8Array
+  const uint8 = new Uint8Array(decodedSVGData.length);
+  for (let i = 0; i < decodedSVGData.length; ++i) {
+    uint8[i] = decodedSVGData.charCodeAt(i);
+  }
+
+  // Create and return the Blob
+  return new Blob([uint8], { type: "image/svg+xml" });
+};
+
 /**
  * Stops a React synthetic event from propagating to ancestor handlers, including
  * Radix Dialog's outside-interaction detection and the editor's own click
@@ -243,7 +258,7 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
   const imgRef = useRef<HTMLImageElement | null>(null);
   const overlayRef = useRef<HTMLDivElement | null>(null);
   const contentRef = useRef<HTMLDivElement | null>(null);
-  const [status, setStatus] = useState<Status>({ lightbox: null, image: null });
+  const [status, dispatch] = useReducer(reducer, initialStatus);
   const [commentsOpen, setCommentsOpen] = useState(false);
   const [commentsRendered, setCommentsRendered] = useState(false);
   const [commentsVisible, setCommentsVisible] = useState(false);
@@ -267,111 +282,39 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
     (img) => img.pos === activeImage.pos
   );
 
-  // Debugging status changes
+  const handleImageLoading = useCallback(
-  // useEffect(() => {
+    () => dispatch({ type: "imageLoading" }),
-  //   console.log(
+    []
-  //     `lstat:${status.lightbox === null ? status.lightbox : LightboxStatus[status.lightbox]}, istat:${status.image === null ? status.image : ImageStatus[status.image]}`
+  );
-  //   );
+  const handleImageLoad = useCallback(
-  // }, [status]);
+    () => dispatch({ type: "imageLoaded" }),
-
+    []
-  useEffect(
+  );
-    () => () => {
+  const handleImageError = useCallback(
-      if (status.lightbox === LightboxStatus.CLOSED) {
+    () => dispatch({ type: "imageErrored" }),
-        onClose();
+    []
-      }
+  );
-    },
+  const handleMinZoom = useCallback(
-    [status.lightbox]
+    () => dispatch({ type: "zoomChanged", zoom: ImageStatus.MIN_ZOOM }),
+    []
+  );
+  const handleZoom = useCallback(
+    () => dispatch({ type: "zoomChanged", zoom: ImageStatus.ZOOMED }),
+    []
+  );
+  const handleMaxZoom = useCallback(
+    () => dispatch({ type: "zoomChanged", zoom: ImageStatus.MAX_ZOOM }),
+    []
   );
 
+  // Keep the latest callbacks in a ref so the close sequence can fire them
+  // without re-running when the caller passes new function identities.
+  const callbacks = useRef({ onUpdate, onClose });
   useEffect(() => {
-    setStatus({
+    callbacks.current = { onUpdate, onClose };
-      lightbox: LightboxStatus.READY_TO_OPEN,
+  });
-      image: status.image,
-    });
-  }, []);
 
-  useEffect(() => {
+  const rememberImagePosition = useCallback(() => {
-    if (status.image === ImageStatus.LOADED) {
-      rememberImagePosition();
-    }
-  }, [status.image]);
-
-  useEffect(() => {
-    if (
-      (status.image === ImageStatus.ERROR ||
-        status.image === ImageStatus.LOADED) &&
-      status.lightbox === LightboxStatus.READY_TO_OPEN
-    ) {
-      setupFadeIn();
-      setupZoomIn();
-      setStatus({
-        lightbox: LightboxStatus.OPENING,
-        image: status.image,
-      });
-    }
-  }, [status.image, status.lightbox]);
-
-  useEffect(() => {
-    if (
-      status.lightbox === LightboxStatus.OPENED &&
-      status.image === ImageStatus.LOADED
-    ) {
-      setStatus({
-        lightbox: LightboxStatus.OPENED,
-        image: ImageStatus.MIN_ZOOM,
-      });
-    }
-  }, [status.lightbox, status.image]);
-
-  useEffect(() => {
-    if (status.lightbox === LightboxStatus.READY_TO_CLOSE) {
-      setupFadeOut();
-      setupZoomOut();
-      setStatus({
-        lightbox: LightboxStatus.CLOSING,
-        image: status.image,
-      });
-    }
-  }, [status.lightbox]);
-
-  useEffect(() => {
-    if (status.lightbox === LightboxStatus.CLOSED) {
-      onUpdate(null);
-    }
-  }, [status.lightbox]);
-
-  useEffect(() => {
-    if (commentsOpen) {
-      setCommentsRendered(true);
-      const frame = window.requestAnimationFrame(() =>
-        setCommentsVisible(true)
-      );
-      return () => window.cancelAnimationFrame(frame);
-    }
-    setCommentsVisible(false);
-    const timer = window.setTimeout(() => setCommentsRendered(false), 200);
-    return () => window.clearTimeout(timer);
-  }, [commentsOpen]);
-
-  useEffect(() => {
-    if (status.image === ImageStatus.MIN_ZOOM) {
-      // It was observed that focus went to `body` as the zoom out button was disabled
-      // upon clicking it. This stopped navigating to next/previous image using arrow keys.
-      // So focusing the content div here to restore the functionality.
-      contentRef.current?.focus();
-    }
-  }, [status.image]);
-
-  // Hide the inline image in the editor while the lightbox zoom transition is
-  // active, otherwise a duplicate is visible behind the fading overlay.
-  useHideElement(
-    activeImage.getElement(),
-    status.lightbox !== null &&
-      status.lightbox !== LightboxStatus.READY_TO_OPEN &&
-      status.lightbox !== LightboxStatus.CLOSED
-  );
-
-  const rememberImagePosition = () => {
     if (imgRef.current) {
       const lightboxImgDOMRect = imgRef.current.getBoundingClientRect();
       const {
@@ -389,9 +332,9 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
         height: lightboxImgHeight,
       };
     }
-  };
+  }, []);
 
-  const setupZoomIn = () => {
+  const setupZoomIn = useCallback(() => {
     if (imgRef.current) {
       // in editor
       const editorImageEl = activeImage.getElement();
@@ -452,9 +395,9 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
         zoomIn: { apply: zoomIn, duration: ANIMATION_DURATION },
       };
     }
-  };
+  }, [activeImage]);
 
-  const setupFadeIn = () => {
+  const setupFadeIn = useCallback(() => {
     const fadeIn = () => keyframes`
                     from { opacity: 0; }
                     to { opacity: 1; }
@@ -464,9 +407,9 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
       fadeIn: { apply: fadeIn, duration: ANIMATION_DURATION },
       fadeOut: undefined,
     };
-  };
+  }, []);
 
-  const setupFadeOut = () => {
+  const setupFadeOut = useCallback(() => {
     const fadeOut = () => keyframes`
               from { opacity: ${overlayRef.current ? window.getComputedStyle(overlayRef.current).opacity : 1}; }
               to { opacity: 0; }
@@ -481,9 +424,9 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
           : ANIMATION_DURATION,
       },
     };
-  };
+  }, []);
 
-  const setupZoomOut = () => {
+  const setupZoomOut = useCallback(() => {
     if (
       imgRef.current &&
       !(
@@ -584,7 +527,98 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
         },
       };
     }
-  };
+  }, [activeImage, status.image, rememberImagePosition]);
+
+  useEffect(() => {
+    dispatch({ type: "mounted" });
+  }, []);
+
+  // Notify the caller once a completed close unmounts the lightbox. The cleanup
+  // captures the status at the time it was registered, so it only fires when
+  // the lightbox is torn down having already reached CLOSED.
+  useEffect(
+    () => () => {
+      if (status.lightbox === LightboxStatus.CLOSED) {
+        callbacks.current.onClose();
+      }
+    },
+    [status.lightbox]
+  );
+
+  // The image position and the opening keyframes are measured after the loaded
+  // image has been laid out, so they cannot be folded into the reducer.
+  useEffect(() => {
+    // MIN_ZOOM as well as LOADED, as the status settles straight to MIN_ZOOM
+    // when navigating to another image with the lightbox already open. Both
+    // mean the image is resting at its natural size, which is what is cached.
+    if (
+      status.image === ImageStatus.LOADED ||
+      status.image === ImageStatus.MIN_ZOOM
+    ) {
+      rememberImagePosition();
+    }
+
+    if (
+      (status.image === ImageStatus.ERROR ||
+        status.image === ImageStatus.LOADED) &&
+      status.lightbox === LightboxStatus.READY_TO_OPEN
+    ) {
+      setupFadeIn();
+      setupZoomIn();
+      dispatch({ type: "openAnimationPrepared" });
+    }
+  }, [
+    status.image,
+    status.lightbox,
+    rememberImagePosition,
+    setupFadeIn,
+    setupZoomIn,
+  ]);
+
+  useEffect(() => {
+    if (status.lightbox === LightboxStatus.READY_TO_CLOSE) {
+      setupFadeOut();
+      setupZoomOut();
+      dispatch({ type: "closeAnimationPrepared" });
+    }
+  }, [status.lightbox, setupFadeOut, setupZoomOut]);
+
+  useEffect(() => {
+    if (status.lightbox === LightboxStatus.CLOSED) {
+      callbacks.current.onUpdate(null);
+    }
+  }, [status.lightbox]);
+
+  useEffect(() => {
+    if (commentsOpen) {
+      setCommentsRendered(true);
+      const frame = window.requestAnimationFrame(() =>
+        setCommentsVisible(true)
+      );
+      return () => window.cancelAnimationFrame(frame);
+    }
+    setCommentsVisible(false);
+    const timer = window.setTimeout(() => setCommentsRendered(false), 200);
+    return () => window.clearTimeout(timer);
+  }, [commentsOpen]);
+
+  useEffect(() => {
+    if (status.image === ImageStatus.MIN_ZOOM) {
+      // It was observed that focus went to `body` as the zoom out button was disabled
+      // upon clicking it. This stopped navigating to next/previous image using arrow keys.
+      // So focusing the content div here to restore the functionality.
+      contentRef.current?.focus();
+    }
+  }, [status.image]);
+
+  // Hide the inline image in the editor while the lightbox zoom transition is
+  // active, otherwise a duplicate is visible behind the fading overlay.
+  useHideElement(
+    activeImage.getElement(),
+    status.lightbox !== null &&
+      status.lightbox !== LightboxStatus.READY_TO_OPEN &&
+      status.lightbox !== LightboxStatus.CLOSED
+  );
 
   const prev = () => {
     if (
@@ -614,76 +648,49 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
     }
   };
 
-  const close = () => {
+  const close = useCallback(() => {
-    if (
+    dispatch({ type: "closeRequested" });
-      status.lightbox === LightboxStatus.OPENING ||
+  }, []);
-      status.lightbox === LightboxStatus.OPENED
-    ) {
-      setStatus({
-        lightbox: LightboxStatus.READY_TO_CLOSE,
-        image: status.image,
-      });
-    }
-  };
 
-  const svgDataURLToBlob = (dataURL: string) => {
+  const downloadImage = useCallback(
-    // Match the SVG data URL format (with or without charset)
+    async (src: string, saveAs: string) => {
-    const match = dataURL.match(
+      let imageBlob;
-      /^data:image\/svg\+xml(?:;charset=utf-8)?,(.*)$/i
+      if (isInternalUrl(src)) {
-    );
+        const image = await fetch(src);
-    if (!match) {
+        imageBlob = await image.blob();
-      return;
+      } else {
-    }
+        // Assuming it's a mermaid svg
+        imageBlob = svgDataURLToBlob(src);
+      }
 
-    const encodedSVGData = match[1];
+      if (!imageBlob) {
-    const decodedSVGData = decodeURIComponent(encodedSVGData);
+        toast.error(t("Unable to download image"));
+        return;
+      }
 
-    // Convert string to Uint8Array
+      const imageURL = URL.createObjectURL(imageBlob);
-    const uint8 = new Uint8Array(decodedSVGData.length);
+      const name = saveAs || "image";
-    for (let i = 0; i < decodedSVGData.length; ++i) {
+      const extension = imageBlob.type.split(/\/|\+/g)[1];
-      uint8[i] = decodedSVGData.charCodeAt(i);
-    }
 
-    // Create and return the Blob
+      // create a temporary link node and click it with our image data
-    return new Blob([uint8], { type: "image/svg+xml" });
+      const link = document.createElement("a");
-  };
+      link.href = imageURL;
+      link.download = `${name}.${extension}`;
+      document.body.appendChild(link);
+      link.click();
 
-  const downloadImage = async (src: string, saveAs: string) => {
+      // cleanup
-    let imageBlob;
+      document.body.removeChild(link);
-    if (isInternalUrl(src)) {
+      URL.revokeObjectURL(imageURL);
-      const image = await fetch(src);
+    },
-      imageBlob = await image.blob();
+    [t]
-    } else {
+  );
-      // Assuming it's a mermaid svg
-      imageBlob = svgDataURLToBlob(src);
-    }
-
-    if (!imageBlob) {
-      toast.error(t("Unable to download image"));
-      return;
-    }
-
-    const imageURL = URL.createObjectURL(imageBlob);
-    const name = saveAs || "image";
-    const extension = imageBlob.type.split(/\/|\+/g)[1];
-
-    // create a temporary link node and click it with our image data
-    const link = document.createElement("a");
-    link.href = imageURL;
-    link.download = `${name}.${extension}`;
-    document.body.appendChild(link);
-    link.click();
-
-    // cleanup
-    document.body.removeChild(link);
-    URL.revokeObjectURL(imageURL);
-  };
 
   const handleDownload = useCallback(() => {
     if (activeImage && status.lightbox === LightboxStatus.OPENED) {
       void downloadImage(activeImage.src, activeImage.alt);
     }
-  }, [activeImage, status.lightbox]);
+  }, [activeImage, status.lightbox, downloadImage]);
 
   const handleKeyDown = (ev: React.KeyboardEvent<HTMLDivElement>) => {
     // Don't intercept keys while typing into an input, textarea, or editor.
@@ -733,26 +740,20 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
         fadeIn: undefined,
         startTime: undefined,
       };
-      setStatus({
+      dispatch({ type: "openAnimationEnded" });
-        lightbox: LightboxStatus.OPENED,
-        image: status.image,
-      });
     } else if (animation.current?.fadeOut) {
-      setStatus({
+      dispatch({ type: "closeAnimationEnded" });
-        lightbox: LightboxStatus.CLOSED,
-        image: null,
-      });
     }
   };
 
   const handleEditDiagram = () => {
-    const { state, dispatch } = editor.view;
+    const { state, dispatch: dispatchTransaction } = editor.view;
 
     // Select the node at the position
     const tr = state.tr.setSelection(
       NodeSelection.create(state.doc, activeImage.pos)
     );
-    dispatch(tr);
+    dispatchTransaction(tr);
     editor.commands.editDiagram();
   };
 
@@ -918,48 +919,18 @@ function Lightbox({ images, activeImage, onUpdate, onClose, readOnly }: Props) {
               ref={imgRef}
               src={activeImage.src}
               alt={activeImage.alt}
-              onLoading={() =>
+              onLoading={handleImageLoading}
-                setStatus({
+              onLoad={handleImageLoad}
-                  lightbox: status.lightbox,
+              onError={handleImageError}
-                  image: ImageStatus.LOADING,
-                })
-              }
-              onLoad={() =>
-                setStatus({
-                  lightbox: status.lightbox,
-                  image: ImageStatus.LOADED,
-                })
-              }
-              onError={() =>
-                setStatus({
-                  lightbox: status.lightbox,
-                  image: ImageStatus.ERROR,
-                })
-              }
               onSwipeRight={prev}
               onSwipeLeft={next}
               onSwipeUp={close}
               onSwipeDown={close}
               status={status}
               animation={animation.current}
-              onMinZoom={() => {
+              onMinZoom={handleMinZoom}
-                setStatus({
+              onZoom={handleZoom}
-                  lightbox: status.lightbox,
+              onMaxZoom={handleMaxZoom}
-                  image: ImageStatus.MIN_ZOOM,
-                });
-              }}
-              onZoom={() =>
-                setStatus({
-                  lightbox: status.lightbox,
-                  image: ImageStatus.ZOOMED,
-                })
-              }
-              onMaxZoom={() =>
-                setStatus({
-                  lightbox: status.lightbox,
-                  image: ImageStatus.MAX_ZOOM,
-                })
-              }
             />
           </ZoomablePannablePinchable>
           {currentImageIndex < images.length - 1 &&
@@ -1073,14 +1044,13 @@ const Image = forwardRef<HTMLImageElement, ImageProps>(function Image_(
 
   useEffect(() => {
     onLoading();
-  }, [src]);
+  }, [src, onLoading]);
 
+  // Anything past loading means there is something to show. This deliberately
+  // does not test for LOADED specifically, as the status can settle straight to
+  // MIN_ZOOM when the lightbox is already open and a new image is navigated to.
   useEffect(() => {
-    if (status.image === null || status.image === ImageStatus.LOADING) {
+    setHidden(status.image === null || status.image === ImageStatus.LOADING);
-      setHidden(true);
-    } else if (status.image === ImageStatus.LOADED) {
-      setHidden(false);
-    }
   }, [status.image]);
 
   return status.image === ImageStatus.ERROR ? (

app/components/LightboxState.test.ts

@@ -0,0 +1,145 @@
+import type { Action } from "./LightboxState";
+import {
+  ImageStatus,
+  LightboxStatus,
+  initialStatus,
+  reducer,
+} from "./LightboxState";
+
+/**
+ * Applies a sequence of actions to the initial status.
+ *
+ * @param actions The actions to apply in order.
+ * @returns The resulting status.
+ */
+const run = (...actions: Action[]) => actions.reduce(reducer, initialStatus);
+
+describe("Lightbox state", () => {
+  it("opens once the image has loaded and the animation has run", () => {
+    const status = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" }
+    );
+
+    expect(status).toEqual({
+      lightbox: LightboxStatus.OPENED,
+      image: ImageStatus.MIN_ZOOM,
+    });
+  });
+
+  it("opens when the image fails to load", () => {
+    const status = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageErrored" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" }
+    );
+
+    expect(status).toEqual({
+      lightbox: LightboxStatus.OPENED,
+      image: ImageStatus.ERROR,
+    });
+  });
+
+  it("settles at minimum zoom when the image loads after opening", () => {
+    const opened = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" }
+    );
+
+    // Navigating to another image while already open.
+    const loading = reducer(opened, { type: "imageLoading" });
+    expect(loading.image).toBe(ImageStatus.LOADING);
+
+    const loaded = reducer(loading, { type: "imageLoaded" });
+    expect(loaded).toEqual({
+      lightbox: LightboxStatus.OPENED,
+      image: ImageStatus.MIN_ZOOM,
+    });
+  });
+
+  it("preserves the lightbox status when the zoom changes", () => {
+    const opened = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" }
+    );
+
+    const zoomed = reducer(opened, {
+      type: "zoomChanged",
+      zoom: ImageStatus.MAX_ZOOM,
+    });
+
+    expect(zoomed).toEqual({
+      lightbox: LightboxStatus.OPENED,
+      image: ImageStatus.MAX_ZOOM,
+    });
+  });
+
+  it("closes through to a final closed status", () => {
+    const opened = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" }
+    );
+
+    const requested = reducer(opened, { type: "closeRequested" });
+    expect(requested.lightbox).toBe(LightboxStatus.READY_TO_CLOSE);
+
+    const closing = reducer(requested, { type: "closeAnimationPrepared" });
+    expect(closing.lightbox).toBe(LightboxStatus.CLOSING);
+
+    expect(reducer(closing, { type: "closeAnimationEnded" })).toEqual({
+      lightbox: LightboxStatus.CLOSED,
+      image: null,
+    });
+  });
+
+  it("ignores a close requested before the lightbox has opened", () => {
+    const readyToOpen = reducer(initialStatus, { type: "mounted" });
+
+    expect(reducer(readyToOpen, { type: "closeRequested" })).toBe(readyToOpen);
+  });
+
+  it("ignores a repeated close request while already closing", () => {
+    const closing = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" },
+      { type: "closeRequested" },
+      { type: "closeAnimationPrepared" }
+    );
+
+    expect(reducer(closing, { type: "closeRequested" })).toBe(closing);
+    expect(reducer(closing, { type: "closeAnimationPrepared" })).toBe(closing);
+  });
+
+  it("does not reopen when a slow image load lands after closing started", () => {
+    const closing = run(
+      { type: "mounted" },
+      { type: "imageLoading" },
+      { type: "imageLoaded" },
+      { type: "openAnimationPrepared" },
+      { type: "openAnimationEnded" },
+      { type: "closeRequested" }
+    );
+
+    const late = reducer(closing, { type: "imageLoaded" });
+
+    expect(late.lightbox).toBe(LightboxStatus.READY_TO_CLOSE);
+    expect(reducer(late, { type: "openAnimationPrepared" })).toBe(late);
+  });
+});

app/components/LightboxState.ts

@@ -0,0 +1,97 @@
+export enum LightboxStatus {
+  READY_TO_OPEN,
+  OPENING,
+  OPENED,
+  READY_TO_CLOSE,
+  CLOSING,
+  CLOSED,
+}
+
+export enum ImageStatus {
+  LOADING,
+  ERROR,
+  LOADED,
+  MIN_ZOOM,
+  MAX_ZOOM,
+  ZOOMED,
+}
+
+export type Status = {
+  lightbox: LightboxStatus | null;
+  image: ImageStatus | null;
+};
+
+export type Action =
+  /** The lightbox mounted and is ready to begin its opening animation. */
+  | { type: "mounted" }
+  /** A new image started loading, either the first one or after navigating. */
+  | { type: "imageLoading" }
+  | { type: "imageLoaded" }
+  | { type: "imageErrored" }
+  /** The zoom level changed, either by the user or by resetting the transform. */
+  | { type: "zoomChanged"; zoom: ImageStatus }
+  /** The opening fade and zoom keyframes have been measured and applied. */
+  | { type: "openAnimationPrepared" }
+  | { type: "openAnimationEnded" }
+  /** The user asked to close, e.g. via Escape, the close button, or a swipe. */
+  | { type: "closeRequested" }
+  /** The closing fade and zoom keyframes have been measured and applied. */
+  | { type: "closeAnimationPrepared" }
+  | { type: "closeAnimationEnded" };
+
+export const initialStatus: Status = { lightbox: null, image: null };
+
+/**
+ * Settles the image at minimum zoom once the lightbox has finished opening and
+ * the image has loaded, whichever of the two happens last.
+ *
+ * @param status The status to settle.
+ * @returns The settled status.
+ */
+function settle(status: Status): Status {
+  return status.lightbox === LightboxStatus.OPENED &&
+    status.image === ImageStatus.LOADED
+    ? { ...status, image: ImageStatus.MIN_ZOOM }
+    : status;
+}
+
+/**
+ * Drives the lightbox open and close choreography. Transitions are guarded so
+ * that events arriving out of order — a slow image load completing after the
+ * user has already started closing, for example — cannot move it backwards.
+ *
+ * @param status The current status.
+ * @param action The action to apply.
+ * @returns The next status.
+ */
+export function reducer(status: Status, action: Action): Status {
+  switch (action.type) {
+    case "mounted":
+      return { ...status, lightbox: LightboxStatus.READY_TO_OPEN };
+    case "imageLoading":
+      return settle({ ...status, image: ImageStatus.LOADING });
+    case "imageLoaded":
+      return settle({ ...status, image: ImageStatus.LOADED });
+    case "imageErrored":
+      return settle({ ...status, image: ImageStatus.ERROR });
+    case "zoomChanged":
+      return { ...status, image: action.zoom };
+    case "openAnimationPrepared":
+      return status.lightbox === LightboxStatus.READY_TO_OPEN
+        ? { ...status, lightbox: LightboxStatus.OPENING }
+        : status;
+    case "openAnimationEnded":
+      return settle({ ...status, lightbox: LightboxStatus.OPENED });
+    case "closeRequested":
+      return status.lightbox === LightboxStatus.OPENING ||
+        status.lightbox === LightboxStatus.OPENED
+        ? { ...status, lightbox: LightboxStatus.READY_TO_CLOSE }
+        : status;
+    case "closeAnimationPrepared":
+      return status.lightbox === LightboxStatus.READY_TO_CLOSE
+        ? { ...status, lightbox: LightboxStatus.CLOSING }
+        : status;
+    case "closeAnimationEnded":
+      return { lightbox: LightboxStatus.CLOSED, image: null };
+  }
+}

app/components/PaginatedList.tsx

@@ -217,6 +217,9 @@ const PaginatedList = <T extends PaginatedItem>({
     if (fetch) {
       void fetchResults();
     }
+    // `fetchResults` changes identity as pagination advances, depending on it
+    // here would re-run the initial fetch for every page.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [fetch]);
 
   // Handle updates to fetch or options

app/components/Sharing/Document/AccessControlList.tsx

@@ -247,15 +247,17 @@ const CollectionSquircle = ({ collection }: { collection: Collection }) => {
 
 function useUsersInCollection(collection?: Collection) {
   const { users, memberships } = useStores();
-  const { request } = useRequest(() =>
+  const fetchMemberships = React.useCallback(
-    memberships.fetchPage({ limit: 1, id: collection!.id })
+    () => memberships.fetchPage({ limit: 1, id: collection!.id }),
+    [memberships, collection]
   );
+  const { request } = useRequest(fetchMemberships);
 
   React.useEffect(() => {
     if (collection && !collection.permission) {
       void request();
     }
-  }, [collection]);
+  }, [collection, request]);
 
   return collection
     ? collection.permission

app/components/Sharing/components/GroupMembersPopover.tsx

@@ -28,8 +28,12 @@ export const GroupMembersPopover = observer(({ group, children }: Props) => {
   const { groupUsers } = useStores();
   const [open, setOpen] = React.useState(false);
 
+  // `orderedData` is an observable dependency, it is what recomputes the list
+  // when group membership changes in the store.
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const members = React.useMemo(
     () => groupUsers.inGroup(group.id),
+    // eslint-disable-next-line react-hooks/exhaustive-deps
     [groupUsers.orderedData, group.id]
   );
 

app/components/Sharing/components/Suggestions.tsx

@@ -116,6 +116,9 @@ export const Suggestions = observer(
             : []),
         ...filtered,
       ];
+      // The store collections listed below are observable dependencies, they
+      // are what recompute the suggestions when the underlying data changes.
+      // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [
       getSuggestionForEmail,
       users,

app/components/Sidebar/Shared.tsx

@@ -57,6 +57,8 @@ function SharedSidebar({ share }: Props) {
 
   useEffect(() => {
     ui.tocVisible = share.showTOC;
+    // Only seed the initial visibility, the user can toggle it afterwards.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   if (!rootNode?.children.length) {

app/components/Sidebar/components/ArchiveLink.tsx

@@ -43,7 +43,7 @@ function ArchiveLink() {
     if (disclosure && isUndefined(expanded)) {
       setExpanded(false);
     }
-  }, [disclosure]);
+  }, [disclosure, expanded]);
 
   useEffect(() => {
     if (expanded) {

app/components/Sidebar/components/SharedWithMe.tsx

@@ -92,6 +92,9 @@ function SharedWithMe() {
         })
       );
     }
+    // `history` is read imperatively, the sidebar context should only be
+    // recalculated when the active document or memberships change.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [
     ui.activeDocumentId,
     locationSidebarContext,

app/editor/components/FloatingToolbar.tsx

@@ -49,7 +49,9 @@ function usePosition({
   const [menuWidth, setMenuWidth] = React.useState(0);
   const menuHeight = 36;
 
-  // Measure the menu width after DOM updates to ensure accurate positioning
+  // Measure the menu width after DOM updates to ensure accurate positioning.
+  // Runs after every render by design, the width comparison prevents a loop.
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   React.useLayoutEffect(() => {
     if (menuRef.current) {
       const width = menuRef.current.offsetWidth;

app/editor/components/SelectionToolbar.tsx

@@ -113,6 +113,9 @@ export function SelectionToolbar(props: Props) {
     } else if (selection.empty) {
       setActiveToolbar(null);
     }
+    // `activeToolbar` is read to decide whether the link toolbar should stay
+    // open, re-running when it changes would fight the user's own selection.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [
     readOnly,
     isActive,
@@ -127,7 +130,7 @@ export function SelectionToolbar(props: Props) {
     if (autoFocusLinkInput && activeToolbar !== Toolbar.Link) {
       setAutoFocusLinkInput(false);
     }
-  }, [activeToolbar]);
+  }, [activeToolbar, autoFocusLinkInput]);
 
   const prevActiveToolbar = React.useRef(activeToolbar);
   React.useLayoutEffect(() => {
@@ -183,7 +186,7 @@ export function SelectionToolbar(props: Props) {
     return () => {
       window.removeEventListener("mouseup", handleClickOutside);
     };
-  }, [isActive, readOnly, view]);
+  }, [isActive, readOnly, view, extensions]);
 
   useEventListener(
     "keydown",

app/editor/components/StickyBlockToolbar.tsx

@@ -94,7 +94,9 @@ const StickyBlockToolbar = React.forwardRef(function StickyBlockToolbar_(
 
   const element = getBlockElement(view);
 
-  // Measure the block relative to the portal's offset parent.
+  // Measure the block relative to the portal's offset parent. Runs after every
+  // render by design, the rect comparison prevents a loop.
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   React.useLayoutEffect(() => {
     const track = trackRef.current;
     if (!element || !track) {

app/editor/components/ToolbarMenu.tsx

@@ -55,6 +55,9 @@ function ToolbarDropdown(props: ToolbarDropdownProps) {
     return resolvedItemChildren
       ? mapMenuItems(resolvedItemChildren, commands, view, state)
       : [];
+    // Menu items are resolved against the editor state at the moment the menu
+    // opens, recomputing on every transaction would rebuild the open menu.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [isOpen, commands]);
 
   const handleCloseAutoFocus = useCallback((ev: Event) => {

app/hooks/useComputed.ts

@@ -11,6 +11,8 @@ export function useComputed<T>(
   callback: () => T,
   inputs: DependencyList = []
 ): T {
+  // The dependency list is supplied by the caller so it cannot be verified here.
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const value = useMemo(() => computed(callback), inputs);
   return value.get();
 }

app/hooks/usePaginatedRequest.ts

@@ -81,6 +81,9 @@ export default function usePaginatedRequest<T = unknown>(
           })
       );
     }
+    // `params` is intentionally omitted, callers pass an object literal which
+    // changes identity on every render.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [offset, fetchLimit, requestFn]);
 
   const next = useCallback(() => {
@@ -100,6 +103,9 @@ export default function usePaginatedRequest<T = unknown>(
           limit: fetchLimit,
         })
     );
+    // `params` and `fetchLimit` are intentionally omitted, resetting pagination
+    // should only be driven by the request function changing.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [requestFn]);
 
   return { data, next, loading, error, page, offset, end };

app/hooks/usePinnedDocuments.ts

@@ -1,4 +1,4 @@
-import { useEffect } from "react";
+import { useCallback, useEffect } from "react";
 import usePersistedState from "~/hooks/usePersistedState";
 import useStores from "./useStores";
 
@@ -13,13 +13,15 @@ export function usePinnedDocuments(urlId: UrlId, collectionId?: string) {
     0
   );
 
-  function getPins() {
+  const getPins = useCallback(
-    return urlId === "home"
+    () =>
-      ? pins.home
+      urlId === "home"
-      : collectionId
+        ? pins.home
-        ? pins.inCollection(collectionId)
+        : collectionId
-        : [];
+          ? pins.inCollection(collectionId)
-  }
+          : [],
+    [urlId, collectionId, pins]
+  );
 
   useEffect(() => {
     void pins
@@ -27,7 +29,7 @@ export function usePinnedDocuments(urlId: UrlId, collectionId?: string) {
       .then(() => {
         setPinsCacheCount(getPins().length);
       });
-  }, [collectionId, pins]);
+  }, [urlId, collectionId, pins, getPins, setPinsCacheCount]);
 
   return {
     count: pinsCacheCount,

app/hooks/useRequest.ts

@@ -59,6 +59,9 @@ export default function useRequest<T = unknown>(
     if (makeRequestOnMount) {
       void request();
     }
+    // Only ever request on mount, later changes to the request function are
+    // surfaced through the returned `request` for the caller to invoke.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   return { data, loading, loaded, error, request };

app/scenes/Collection/index.tsx

@@ -115,6 +115,9 @@ const CollectionScene = observer(function CollectionScene_() {
     }
 
     void fetchData();
+    // Fetched once on mount, the slug in `id` also changes when the collection
+    // is renamed which must not trigger a refetch.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   useEffect(() => {

app/scenes/Developer/Changesets.tsx

@@ -14,7 +14,7 @@ import useStores from "~/hooks/useStores";
 import usePersistedState from "~/hooks/usePersistedState";
 import Scrollable from "~/components/Scrollable";
 import Switch from "~/components/Switch";
-import { action } from "mobx";
+import { runInAction } from "mobx";
 import { ChangesetHelper } from "@shared/editor/lib/ChangesetHelper";
 
 /**
@@ -39,8 +39,8 @@ function Changesets() {
    * This ensures that MobX reactions in RevisionViewer and the model computed properties
    * (like `changeset`) are triggered correctly.
    */
-  React.useEffect(
+  React.useEffect(() => {
-    action(() => {
+    runInAction(() => {
       stores.revisions.data.clear();
       stores.documents.data.clear();
 
@@ -82,9 +82,8 @@ function Changesets() {
         createdAt: "2024-01-02T12:00:00.000Z",
         data: selectedExample.after,
       });
-    }),
+    });
-    [selectedExample, id]
+  }, [selectedExample, id]);
-  );
 
   const mockDocument = stores.documents.get("mock-document-id");
   const mockDiffRevision = stores.revisions.get("mock-diff-revision-" + id);

app/scenes/Document/components/KeyboardShortcutsButton.tsx

@@ -29,6 +29,9 @@ function KeyboardShortcutsButton() {
     if (shortcutsQuery !== null) {
       handleOpenKeyboardShortcuts(shortcutsQuery);
     }
+    // Only the query param should open the guide, re-running when the handler
+    // identity changes would reopen an already dismissed dialog.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [shortcutsQuery]);
 
   return (

app/scenes/Document/components/MultiplayerEditor.tsx

@@ -231,6 +231,9 @@ function MultiplayerEditor(
       setRemoteProvider(undefined);
       ui.setMultiplayerStatus(undefined, undefined);
     };
+    // `token` is intentionally omitted, it is only read when establishing the
+    // connection and a refreshed token must not tear down the provider.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [
     history,
     t,

app/scenes/Search/Search.tsx

@@ -68,9 +68,16 @@ function Search() {
   const userId = params.get("userId") ?? "";
   const documentId = params.get("documentId") ?? undefined;
   const dateFilter = (params.get("dateFilter") as TDateFilter) ?? "";
-  const statusFilter = params.getAll("statusFilter")?.length
+  // Keyed on the serialized value so the array keeps a stable identity between
-    ? (params.getAll("statusFilter") as TStatusFilter[])
+  // renders and can be used directly as a dependency.
-    : [TStatusFilter.Published, TStatusFilter.Draft];
+  const statusFilterKey = params.getAll("statusFilter").join(",");
+  const statusFilter = React.useMemo(
+    () =>
+      statusFilterKey
+        ? (statusFilterKey.split(",") as TStatusFilter[])
+        : [TStatusFilter.Published, TStatusFilter.Draft],
+    [statusFilterKey]
+  );
   const titleFilter = isTruthyQueryValue(params.get("titleFilter"));
   const sort = (params.get("sort") as TSortFilter) ?? "";
   const direction = (params.get("direction") as TDirectionFilter) ?? "";
@@ -103,7 +110,7 @@ function Search() {
     }),
     [
       query,
-      JSON.stringify(statusFilter),
+      statusFilter,
       collectionId,
       userId,
       dateFilter,

app/scenes/Settings/Application.tsx

@@ -42,13 +42,17 @@ const LoadingState = observer(function LoadingState() {
   const { id } = useParams<{ id: string }>();
   const { oauthClients } = useStores();
   const oauthClient = oauthClients.get(id);
-  const { request } = useRequest(() => oauthClients.fetch(id));
+  const fetchOAuthClient = useCallback(
+    () => oauthClients.fetch(id),
+    [oauthClients, id]
+  );
+  const { request } = useRequest(fetchOAuthClient);
 
   useEffect(() => {
     if (!oauthClient) {
       void request();
     }
-  }, [oauthClient]);
+  }, [oauthClient, request]);
 
   if (!oauthClient) {
     return <LoadingIndicator />;

shared/editor/components/Embed.tsx

@@ -22,25 +22,14 @@ const Embed = (props: Props) => {
   const naturalHeight = 400;
   const isResizable = !!onChangeSize && !embedsDisabled;
 
-  const { width, height, setSize, handlePointerDown, dragging } = useDragResize(
+  const { width, height, handlePointerDown, dragging } = useDragResize({
-    {
+    width: node.attrs.width ?? naturalWidth,
-      width: node.attrs.width ?? naturalWidth,
+    height: node.attrs.height ?? naturalHeight,
-      height: node.attrs.height ?? naturalHeight,
+    naturalWidth,
-      naturalWidth,
+    naturalHeight,
-      naturalHeight,
+    onChangeSize,
-      onChangeSize,
+    ref,
-      ref,
+  });
-    }
-  );
-
-  React.useEffect(() => {
-    if (node.attrs.height && node.attrs.height !== height) {
-      setSize({
-        width: node.attrs.width,
-        height: node.attrs.height,
-      });
-    }
-  }, [node.attrs.height]);
 
   const style: React.CSSProperties = {
     width: width || "100%",

shared/editor/components/Image.tsx

@@ -94,21 +94,15 @@ const Image = (props: Props) => {
   const [naturalHeight, setNaturalHeight] = React.useState(node.attrs.height);
   const lastTapTimeRef = React.useRef(0);
   const ref = React.useRef<HTMLDivElement>(null);
-  const {
+  const { width, height, handlePointerDown, handleDoubleClick, dragging } =
-    width,
+    useDragResize({
-    height,
+      width: node.attrs.width ?? naturalWidth,
-    setSize,
+      height: node.attrs.height ?? naturalHeight,
-    handlePointerDown,
+      naturalWidth,
-    handleDoubleClick,
+      naturalHeight,
-    dragging,
+      onChangeSize,
-  } = useDragResize({
+      ref,
-    width: node.attrs.width ?? naturalWidth,
+    });
-    height: node.attrs.height ?? naturalHeight,
-    naturalWidth,
-    naturalHeight,
-    onChangeSize,
-    ref,
-  });
 
   const isFullWidth = layoutClass === "full-width";
   const isInlineIcon = isInlineImageIcon({ layoutClass, width, error });
@@ -117,15 +111,6 @@ const Image = (props: Props) => {
 
   const className = imageClassName({ layoutClass, width, error });
 
-  React.useEffect(() => {
-    if (node.attrs.width && node.attrs.width !== width) {
-      setSize({
-        width: node.attrs.width,
-        height: node.attrs.height,
-      });
-    }
-  }, [node.attrs.width]);
-
   const sanitizedSrc = sanitizeImageSrc(src);
   const linkMarkType = props.view.state.schema.marks.link;
   const imgLink =
@@ -261,16 +246,11 @@ const Image = (props: Props) => {
                 // seen and is not sized to 0px
                 const nw = (ev.target as HTMLImageElement).naturalWidth || 300;
                 const nh = (ev.target as HTMLImageElement).naturalHeight;
+                // When no width is set on the node the natural size is what the
+                // image is displayed at, so it feeds straight into useDragResize.
                 setNaturalWidth(nw);
                 setNaturalHeight(nh);
                 setLoaded(true);
-
-                if (!node.attrs.width) {
-                  setSize((state) => ({
-                    ...state,
-                    width: nw,
-                  }));
-                }
               }}
               onClick={handleImageClick}
               onTouchStart={handleImageTouchStart}

shared/editor/components/PDF.tsx

@@ -34,7 +34,7 @@ export default function PdfViewer(props: Props) {
   const embedRef = useRef<HTMLEmbedElement>(null);
   const debounceTimerRef = useRef<NodeJS.Timeout | null>(null);
 
-  const { width, setSize, handlePointerDown, dragging } = useDragResize({
+  const { width, handlePointerDown, dragging } = useDragResize({
     width: node.attrs.width,
     height: node.attrs.height,
     naturalWidth,
@@ -43,16 +43,6 @@ export default function PdfViewer(props: Props) {
     ref,
   });
 
-  useEffect(() => {
-    if (node.attrs.width && node.attrs.width !== width) {
-      setSize({
-        width: node.attrs.width,
-        height: node.attrs.height,
-      });
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [node.attrs.width]);
-
   // force embed to reload, so the content fits the new size.
   useEffect(() => {
     // firefox handles resizing on its own

shared/editor/components/Video.tsx

@@ -18,30 +18,15 @@ export default function Video(props: Props) {
   const ref = React.useRef<HTMLDivElement>(null);
   const isResizable = !!onChangeSize;
 
-  const {
+  const { width, height, handlePointerDown, handleDoubleClick, dragging } =
-    width,
+    useDragResize({
-    height,
+      width: node.attrs.width ?? naturalWidth,
-    setSize,
+      height: node.attrs.height ?? naturalHeight,
-    handlePointerDown,
+      naturalWidth,
-    handleDoubleClick,
+      naturalHeight,
-    dragging,
+      onChangeSize,
-  } = useDragResize({
+      ref,
-    width: node.attrs.width ?? naturalWidth,
+    });
-    height: node.attrs.height ?? naturalHeight,
-    naturalWidth,
-    naturalHeight,
-    onChangeSize,
-    ref,
-  });
-
-  React.useEffect(() => {
-    if (node.attrs.width && node.attrs.width !== width) {
-      setSize({
-        width: node.attrs.width,
-        height: node.attrs.height,
-      });
-    }
-  }, [node.attrs.width]);
 
   const style: React.CSSProperties = {
     width: width || "auto",

shared/editor/components/hooks/useDragResize.ts

@@ -46,8 +46,6 @@ type ReturnValue = {
   ) => (event: React.PointerEvent<HTMLDivElement>) => void;
   /** Event handler for double-click event on the resize handle. */
   handleDoubleClick: () => void;
-  /** Handler to set the new size of the element from outside. */
-  setSize: React.Dispatch<React.SetStateAction<SizeState>>;
   /** Whether the element is currently being resized. */
   dragging: DragDirection | undefined;
   /** The current width of the element. */
@@ -59,9 +57,9 @@ type ReturnValue = {
 type Params = {
   /** Callback triggered when the image is resized */
   onChangeSize?: undefined | ((size: SizeState) => void);
-  /** The initial width of the element. */
+  /** The committed width of the element, this is the source of truth. */
   width: number;
-  /** The initial height of the element. */
+  /** The committed height of the element, this is the source of truth. */
   height: number;
   /** The natural width of the element. */
   naturalWidth: number;
@@ -88,16 +86,39 @@ export default function useDragResize(props: Params): ReturnValue {
     isCentered = true,
   } = props;
 
-  const [size, setSize] = React.useState<SizeState>({
+  // The committed size passed in through props is the source of truth. `draft`
+  // holds the size being previewed while resizing and is released once the
+  // committed size arrives back through props, so the element never flashes at
+  // its previous size between the drag ending and the change round-tripping.
+  const [draft, setDraft] = React.useState<SizeState | null>(null);
+  const [committed, setCommitted] = React.useState<SizeState>({
     width: props.width,
     height: props.height,
   });
   const [maxWidth, setMaxWidth] = React.useState(Infinity);
   const [offset, setOffset] = React.useState({ x: 0, y: 0 });
-  const [sizeAtDragStart, setSizeAtDragStart] = React.useState(size);
+  const [sizeAtDragStart, setSizeAtDragStart] = React.useState<SizeState>({
+    width: props.width,
+    height: props.height,
+  });
   const [dragging, setDragging] = React.useState<DragDirection>();
   const isResizable = !!onChangeSize;
 
+  // Release the draft whenever the committed size changes, whether that's this
+  // element's own resize landing or an external change such as undo, a reset,
+  // or a collaborator resizing the same node. Skipped while dragging so a
+  // remote change cannot yank the element out from under the pointer.
+  if (
+    !dragging &&
+    (!Object.is(committed.width, props.width) ||
+      !Object.is(committed.height, props.height))
+  ) {
+    setCommitted({ width: props.width, height: props.height });
+    setDraft(null);
+  }
+
+  const size = draft ?? { width: props.width, height: props.height };
+
   // Mirror the latest size into a ref so handlePointerUp can read it without
   // re-binding listeners on every pointermove that updates size.
   const sizeRef = React.useRef(size);
@@ -175,7 +196,7 @@ export default function useDragResize(props: Params): ReturnValue {
             : undefined
           : sizeAtDragStart.height;
 
-        setSize({
+        setDraft({
           width: nextWidth,
           height: nextHeight,
         });
@@ -197,21 +218,23 @@ export default function useDragResize(props: Params): ReturnValue {
         const heightOnGrid = Math.round(newHeight / gridHeight) * gridHeight;
         const nextHeight = Math.max(heightOnGrid, minHeight ?? 50);
 
-        setSize((state) => {
+        // Vertical-only drags never adjust the width, so it is carried over
-          const nextState = {
+        // from the current size rather than recomputed.
-            ...state,
+        const nextState = {
-            height: nextHeight,
+          width: sizeRef.current.width,
-          };
+          height: nextHeight,
-          window.dispatchEvent(
+        };
-            new CustomEvent("media-drag-resize", {
+
-              detail: {
+        setDraft(nextState);
-                ...nextState,
+
-                isDragging: true,
+        window.dispatchEvent(
-              },
+          new CustomEvent("media-drag-resize", {
-            })
+            detail: {
-          );
+              ...nextState,
-          return nextState;
+              isDragging: true,
-        });
+            },
+          })
+        );
       }
     },
     [
@@ -224,6 +247,7 @@ export default function useDragResize(props: Params): ReturnValue {
       naturalHeight,
       minHeight,
       constrainWidth,
+      isCentered,
     ]
   );
 
@@ -254,7 +278,7 @@ export default function useDragResize(props: Params): ReturnValue {
         event.preventDefault();
         event.stopPropagation();
 
-        setSize(sizeAtDragStart);
+        setDraft(sizeAtDragStart);
         setDragging(undefined);
 
         window.dispatchEvent(
@@ -280,7 +304,7 @@ export default function useDragResize(props: Params): ReturnValue {
       width: naturalWidth,
       height: naturalHeight,
     };
-    setSize(newSize);
+    setDraft(newSize);
     onChangeSize?.(newSize);
   };
 
@@ -346,7 +370,6 @@ export default function useDragResize(props: Params): ReturnValue {
     handlePointerDown,
     handleDoubleClick,
     dragging,
-    setSize,
     width: size.width,
     height: size.height,
   };