Revert "refactor(editor-viewer): simplify loading/save flow with timeout fallback (#5905)"

This reverts commit c84bb91f4ad48c7a83d6e0ceb9370637b149c5ea.

src/components/profile/editor-viewer.tsx

@@ -19,7 +19,7 @@ import { useLockFn } from "ahooks";
 import * as monaco from "monaco-editor";
 import { configureMonacoYaml } from "monaco-yaml";
 import { nanoid } from "nanoid";
-import { ReactNode, useEffect, useMemo, useRef, useState } from "react";
+import { ReactNode, useEffect, useRef, useState } from "react";
 import { useTranslation } from "react-i18next";
 import pac from "types-pac/pac.d.ts?raw";
 
@@ -35,8 +35,10 @@ type Language = "yaml" | "javascript" | "css";
 interface Props<T extends Language> {
   open: boolean;
   title?: string | ReactNode;
+  // Initial content loader: prefer passing a stable function. A plain Promise is supported,
+  // but it won't trigger background refreshes and should be paired with a stable `dataKey`.
   initialData: Promise<string> | (() => Promise<string>);
-  // Logical document id; used to build a stable model path.
+  // Logical document id; reloads when this or language changes.
   dataKey?: string | number;
   readOnly?: boolean;
   language: T;
@@ -45,16 +47,16 @@ interface Props<T extends Language> {
   onClose: () => void;
 }
 
-const LOAD_TIMEOUT_MS = 15000;
-
 let initialized = false;
 const monacoInitialization = () => {
   if (initialized) return;
 
+  // YAML worker setup
   configureMonacoYaml(monaco, {
     validate: true,
     enableSchemaRequest: true,
   });
+  // PAC type definitions for JS suggestions
   monaco.typescript.javascriptDefaults.addExtraLib(pac, "pac.d.ts");
 
   initialized = true;
@@ -79,139 +81,237 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
 
   const resolvedTitle = title ?? t("profiles.components.menu.editFile");
 
-  const editorRef = useRef<monaco.editor.IStandaloneCodeEditor | null>(null);
+  const editorRef = useRef<monaco.editor.IStandaloneCodeEditor>(undefined);
-  const prevData = useRef<string>("");
+  const prevData = useRef<string | undefined>("");
-  const currData = useRef<string>("");
+  const currData = useRef<string | undefined>("");
-  const userEditedRef = useRef(false);
+  // Hold the latest loader without making effects depend on its identity
-  const loadIdRef = useRef(0);
   const initialDataRef = useRef<Props<T>["initialData"]>(initialData);
-  const instanceIdRef = useRef<string>(nanoid());
+  // Track mount/open state to prevent setState after unmount/close
-
+  const isMountedRef = useRef(true);
+  const openRef = useRef(open);
+  useEffect(() => {
+    openRef.current = open;
+  }, [open]);
+  useEffect(() => {
+    isMountedRef.current = true;
+    return () => {
+      isMountedRef.current = false;
+    };
+  }, []);
   const [initialText, setInitialText] = useState<string | null>(null);
-  const [canSave, setCanSave] = useState(false);
+  const [modelPath, setModelPath] = useState<string>("");
-
+  const modelChangeDisposableRef = useRef<monaco.IDisposable | null>(null);
-  const modelPath = useMemo(() => {
+  // Unique per-component instance id to avoid shared Monaco models across dialogs
-    const key = dataKey ?? "editor";
+  const instanceIdRef = useRef<string>(nanoid());
-    return `${key}.${instanceIdRef.current}.${language}`;
+  // Disable actions while loading or before modelPath is ready
-  }, [dataKey, language]);
+  const isLoading = initialText === null || !modelPath;
-
+  // Track if background refresh failed; offer a retry action in UI
-  const isLoading = open && initialText === null;
+  const [refreshFailed, setRefreshFailed] = useState<unknown | null>(null);
-
+  // Skip the first background refresh triggered by [open, modelPath, dataKey]
+  // to avoid double-invoking the loader right after the initial load.
+  const skipNextRefreshRef = useRef(false);
+  // Monotonic token to cancel stale background refreshes
+  const reloadTokenRef = useRef(0);
+  // Track whether the editor has a usable baseline (either loaded or fallback).
+  // This avoids saving before the model/path are ready, while still allowing recovery
+  // when the initial load fails but an empty buffer is presented.
+  const [hasLoadedOnce, setHasLoadedOnce] = useState(false);
+  // Editor should only be read-only when explicitly requested by prop.
+  // A refresh/load failure must not lock the editor to allow manual recovery.
+  const effectiveReadOnly = readOnly;
+  // Keep ref in sync with prop without triggering loads
   useEffect(() => {
     initialDataRef.current = initialData;
   }, [initialData]);
-
+  // Background refresh: when the dialog/model is ready and the underlying resource key changes,
+  // try to refresh content (only if user hasn't typed). Do NOT depend on `initialData` function
+  // identity because callers often pass inline lambdas that change every render.
   useEffect(() => {
     if (!open) return;
-
+    // Only attempt after initial model is ready to avoid racing the initial load
-    let cancelled = false;
+    if (!modelPath) return;
-    const loadId = ++loadIdRef.current;
+    // Avoid immediate double-load on open: the initial load has just completed.
-    userEditedRef.current = false;
+    if (skipNextRefreshRef.current) {
-    prevData.current = "";
+      skipNextRefreshRef.current = false;
-    currData.current = "";
-    // eslint-disable-next-line @eslint-react/hooks-extra/no-direct-set-state-in-use-effect
-    setInitialText(null);
-    // eslint-disable-next-line @eslint-react/hooks-extra/no-direct-set-state-in-use-effect
-    setCanSave(false);
-
-    let didTimeout = false;
-    const timeoutId = window.setTimeout(() => {
-      didTimeout = true;
-      if (cancelled || loadId !== loadIdRef.current) return;
-      prevData.current = "";
-      currData.current = "";
-      setInitialText("");
-      setCanSave(false);
-      showNotice.error("shared.feedback.notifications.common.refreshFailed");
-    }, LOAD_TIMEOUT_MS);
-
-    const dataPromise = Promise.resolve().then(() => {
-      const dataSource = initialDataRef.current;
-      if (typeof dataSource === "function") {
-        return (dataSource as () => Promise<string>)();
-      }
-      return dataSource ?? "";
-    });
-
-    dataPromise
-      .then((data) => {
-        if (cancelled || loadId !== loadIdRef.current) return;
-        clearTimeout(timeoutId);
-
-        if (userEditedRef.current) {
-          setCanSave(true);
       return;
     }
-
+    // Only meaningful when a callable loader is provided (plain Promise cannot be "recalled")
-        const next = data ?? "";
+    if (typeof initialDataRef.current === "function") {
+      void reloadLatest();
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [open, modelPath, dataKey]);
+  // Helper to (soft) reload latest source and apply only if the user hasn't typed yet
+  const reloadLatest = useLockFn(async () => {
+    // Snapshot the model/doc identity and bump a token so older calls can't win
+    const myToken = ++reloadTokenRef.current;
+    const expectedModelPath = modelPath;
+    const expectedKey = dataKey;
+    if (isMountedRef.current && openRef.current) {
+      // Clear previous error (UI hint) at the start of a new attempt
+      setRefreshFailed(null);
+    }
+    try {
+      const src = initialDataRef.current;
+      const promise =
+        typeof src === "function"
+          ? (src as () => Promise<string>)()
+          : (src ?? Promise.resolve(""));
+      const next = await promise;
+      // Abort if component/dialog state changed meanwhile:
+      // - unmounted or closed
+      // - document switched (modelPath/dataKey no longer match)
+      // - a newer reload was started
+      if (
+        !isMountedRef.current ||
+        !openRef.current ||
+        expectedModelPath !== modelPath ||
+        expectedKey !== dataKey ||
+        myToken !== reloadTokenRef.current
+      ) {
+        return;
+      }
+      // Only update when untouched and value changed
+      const userUntouched = currData.current === prevData.current;
+      if (userUntouched && next !== prevData.current) {
         prevData.current = next;
         currData.current = next;
-
+        editorRef.current?.setValue(next);
-        if (didTimeout) {
-          if (editorRef.current) {
-            editorRef.current.setValue(next);
-          } else {
-            setInitialText(next);
       }
-        } else {
+      // Ensure any previous error state is cleared after a successful refresh
-          setInitialText(next);
+      if (isMountedRef.current && openRef.current) {
+        setRefreshFailed(null);
       }
-        setCanSave(true);
+      // If we previously failed to load, a successful refresh establishes a valid baseline
-      })
+      if (isMountedRef.current && openRef.current) {
-      .catch((err) => {
+        setHasLoadedOnce(true);
-        if (cancelled || loadId !== loadIdRef.current) return;
-        clearTimeout(timeoutId);
-
-        if (!didTimeout) {
-          setInitialText("");
       }
-        if (!userEditedRef.current) {
+    } catch (err) {
-          setCanSave(false);
+      // Only report if still mounted/open and this call is the latest
+      if (
+        isMountedRef.current &&
+        openRef.current &&
+        myToken === reloadTokenRef.current
+      ) {
+        setRefreshFailed(err ?? true);
+        showNotice.error(
+          "shared.feedback.notifications.common.refreshFailed",
+          err,
+        );
       }
-        if (!didTimeout) {
-          showNotice.error(err);
     }
   });
 
+  const beforeMount = () => {
+    monacoInitialization();
+  };
+
+  // Prepare initial content and a stable model path for monaco-react
+  /* eslint-disable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */
+  useEffect(() => {
+    if (!open) return;
+    let cancelled = false;
+    // Clear state up-front to avoid showing stale content while loading
+    setInitialText(null);
+    setModelPath("");
+    // Clear any stale refresh error when starting a new load
+    setRefreshFailed(null);
+    // Reset initial-load success flag on open/start
+    setHasLoadedOnce(false);
+    // We will perform an explicit initial load below; skip the first background refresh.
+    skipNextRefreshRef.current = true;
+    prevData.current = undefined;
+    currData.current = undefined;
+
+    (async () => {
+      try {
+        const dataSource = initialDataRef.current;
+        const dataPromise =
+          typeof dataSource === "function"
+            ? (dataSource as () => Promise<string>)()
+            : (dataSource ?? Promise.resolve(""));
+        const data = await dataPromise;
+        if (cancelled) return;
+        prevData.current = data;
+        currData.current = data;
+
+        setInitialText(data);
+        // Build a stable model path and avoid "undefined" in the name
+        const pathParts = [String(dataKey ?? nanoid()), instanceIdRef.current];
+        pathParts.push(language);
+
+        setModelPath(pathParts.join("."));
+        // Successful initial load should clear any previous refresh error flag
+        setRefreshFailed(null);
+        // Mark that we have a valid baseline content
+        setHasLoadedOnce(true);
+      } catch (err) {
+        if (cancelled) return;
+        // Notify the error and still show an empty editor so the user isn't stuck
+        showNotice.error(err);
+
+        // Align refs with fallback text after a load failure
+        prevData.current = "";
+        currData.current = "";
+
+        setInitialText("");
+        const pathParts = [String(dataKey ?? nanoid()), instanceIdRef.current];
+        pathParts.push(language);
+
+        setModelPath(pathParts.join("."));
+        // Mark refresh failure so users can retry
+        setRefreshFailed(err ?? true);
+        // Initial load failed; keep `hasLoadedOnce` false to prevent accidental save
+        // of an empty buffer. It will be enabled on successful refresh or first edit.
+        setHasLoadedOnce(false);
+      }
+    })();
+
     return () => {
       cancelled = true;
-      clearTimeout(timeoutId);
     };
   }, [open, dataKey, language]);
+  /* eslint-enable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */
 
-  const beforeMount = () => {
+  const onMount = async (editor: monaco.editor.IStandaloneCodeEditor) => {
-    try {
-      monacoInitialization();
-    } catch (err) {
-      showNotice.error(err);
-    }
-  };
-
-  const onMount = (editor: monaco.editor.IStandaloneCodeEditor) => {
     editorRef.current = editor;
+    // Dispose previous model when switching (monaco-react creates a fresh model when `path` changes)
+    modelChangeDisposableRef.current?.dispose();
+    modelChangeDisposableRef.current = editor.onDidChangeModel((e) => {
+      if (e.oldModelUrl) {
+        const oldModel = monaco.editor.getModel(e.oldModelUrl);
+        oldModel?.dispose();
+      }
+    });
+    // No refresh on mount; doing so would double-load.
+    // Background refreshes are handled by the [open, modelPath, dataKey] effect.
   };
 
-  const handleChange = (value?: string) => {
+  const handleChange = useLockFn(async (value?: string) => {
     try {
-      const next = value ?? editorRef.current?.getValue() ?? "";
+      currData.current = value ?? editorRef.current?.getValue();
-      currData.current = next;
+      onChange?.(prevData.current, currData.current);
-      userEditedRef.current = true;
+      // If the initial load failed, allow saving after the user makes an edit.
-      if (!readOnly) {
+      if (!hasLoadedOnce) {
-        setCanSave(true);
+        setHasLoadedOnce(true);
       }
-      onChange?.(prevData.current, next);
     } catch (err) {
       showNotice.error(err);
     }
-  };
+  });
 
   const handleSave = useLockFn(async () => {
     try {
-      if (!readOnly && canSave) {
+      // Disallow saving if initial content never loaded successfully to avoid accidental overwrite
-        if (!editorRef.current) return;
+      if (!readOnly && hasLoadedOnce) {
+        // Guard: if the editor/model hasn't mounted, bail out
+        if (!editorRef.current) {
+          return;
+        }
         currData.current = editorRef.current.getValue();
         if (onSave) {
           await onSave(prevData.current, currData.current);
+          // If save succeeds, align prev with current
           prevData.current = currData.current;
         }
       }
@@ -221,15 +321,17 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
     }
   });
 
+  // Explicit paste action: works even when Monaco's context-menu paste cannot read clipboard.
   const handlePaste = useLockFn(async () => {
     try {
-      if (!editorRef.current || readOnly) return;
+      if (!editorRef.current || effectiveReadOnly) return;
       const text = await navigator.clipboard.readText();
       if (!text) return;
       const editor = editorRef.current;
       const model = editor.getModel();
       const selections = editor.getSelections();
       if (!model || !selections || selections.length === 0) return;
+      // Group edits to allow single undo step
       editor.pushUndoStop();
       editor.executeEdits(
         "explicit-paste",
@@ -259,6 +361,8 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
       appWindow
         .isMaximized()
         .then((maximized) => setIsMaximized(() => maximized));
+      // Ensure Monaco recalculates layout after window resize/maximize/restore.
+      // automaticLayout is not always sufficient when the parent dialog resizes.
       try {
         editorRef.current?.layout();
       } catch {}
@@ -267,10 +371,13 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
 
     return () => {
       unlistenResized.then((fn) => fn());
+      // Clean up editor and model to avoid leaks
       const model = editorRef.current?.getModel();
       editorRef.current?.dispose();
       model?.dispose();
-      editorRef.current = null;
+      modelChangeDisposableRef.current?.dispose();
+      modelChangeDisposableRef.current = null;
+      editorRef.current = undefined;
     };
   }, []);
 
@@ -287,6 +394,7 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
       <DialogContent
         sx={{
           width: "auto",
+          // Give the editor a scrollable height (even in nested dialogs)
           height: "calc(100vh - 185px)",
           display: "flex",
           flexDirection: "column",
@@ -294,8 +402,46 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
         }}
       >
         <div style={{ position: "relative", flex: "1 1 auto", minHeight: 0 }}>
+          {/* Show overlay while loading or until modelPath is ready */}
           <BaseLoadingOverlay isLoading={isLoading} />
-          {initialText !== null && (
+          {/* Background refresh failure helper */}
+          {!!refreshFailed && (
+            <div
+              style={{
+                position: "absolute",
+                top: 8,
+                right: 10,
+                zIndex: 2,
+                display: "flex",
+                gap: 8,
+                alignItems: "center",
+                pointerEvents: "auto",
+              }}
+            >
+              <span
+                style={{
+                  color: "var(--mui-palette-warning-main, #ed6c02)",
+                  background: "rgba(237,108,2,0.1)",
+                  border: "1px solid rgba(237,108,2,0.35)",
+                  borderRadius: 6,
+                  padding: "2px 8px",
+                  fontSize: 12,
+                  lineHeight: "20px",
+                  userSelect: "text",
+                }}
+              >
+                {t("shared.feedback.notifications.common.refreshFailed")}
+              </span>
+              <Button
+                size="small"
+                variant="outlined"
+                onClick={() => reloadLatest()}
+              >
+                {t("shared.actions.retry")}
+              </Button>
+            </div>
+          )}
+          {initialText !== null && modelPath && (
             <MonacoEditor
               height="100%"
               path={modelPath}
@@ -311,7 +457,7 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
                   enabled: document.documentElement.clientWidth >= 1500,
                 },
                 mouseWheelZoom: true,
-                readOnly,
+                readOnly: effectiveReadOnly,
                 readOnlyMessage: {
                   value: t("profiles.modals.editor.messages.readOnly"),
                 },
@@ -322,7 +468,7 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
                   other: true,
                 },
                 padding: {
-                  top: 33,
+                  top: 33, // Top padding to prevent snippet overlap
                 },
                 fontFamily: `Fira Code, JetBrains Mono, Roboto Mono, "Source Code Pro", Consolas, Menlo, Monaco, monospace, "Courier New", "Apple Color Emoji"${
                   getSystem() === "windows" ? ", twemoji mozilla" : ""
@@ -380,6 +526,7 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
                     .then((maximized) => setIsMaximized(maximized)),
                 )
                 .finally(() => {
+                  // Nudge a layout in case the resize event batching lags behind
                   try {
                     editorRef.current?.layout();
                   } catch {}
@@ -399,7 +546,7 @@ export const EditorViewer = <T extends Language>(props: Props<T>) => {
           <Button
             onClick={handleSave}
             variant="contained"
-            disabled={isLoading || !canSave}
+            disabled={isLoading || !hasLoadedOnce}
           >
             {t("shared.actions.save")}
           </Button>