From 2515deefed3d71df9abaa2d37ed4d9c05efab304 Mon Sep 17 00:00:00 2001 From: Slinetrac Date: Sun, 21 Dec 2025 20:28:38 +0800 Subject: [PATCH] Revert "refactor(editor-viewer): simplify loading/save flow with timeout fallback (#5905)" This reverts commit c84bb91f4ad48c7a83d6e0ceb9370637b149c5ea. --- src/components/profile/editor-viewer.tsx | 369 ++++++++++++++++------- 1 file changed, 258 insertions(+), 111 deletions(-) diff --git a/src/components/profile/editor-viewer.tsx b/src/components/profile/editor-viewer.tsx index e1ae9b723..99d645121 100644 --- a/src/components/profile/editor-viewer.tsx +++ b/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 { 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 | (() => Promise); - // 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 { 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 = (props: Props) => { const resolvedTitle = title ?? t("profiles.components.menu.editFile"); - const editorRef = useRef(null); - const prevData = useRef(""); - const currData = useRef(""); - const userEditedRef = useRef(false); - const loadIdRef = useRef(0); + const editorRef = useRef(undefined); + const prevData = useRef(""); + const currData = useRef(""); + // Hold the latest loader without making effects depend on its identity const initialDataRef = useRef["initialData"]>(initialData); - const instanceIdRef = useRef(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(null); - const [canSave, setCanSave] = useState(false); - - const modelPath = useMemo(() => { - const key = dataKey ?? "editor"; - return `${key}.${instanceIdRef.current}.${language}`; - }, [dataKey, language]); - - const isLoading = open && initialText === null; - + const [modelPath, setModelPath] = useState(""); + const modelChangeDisposableRef = useRef(null); + // Unique per-component instance id to avoid shared Monaco models across dialogs + const instanceIdRef = useRef(nanoid()); + // Disable actions while loading or before modelPath is ready + const isLoading = initialText === null || !modelPath; + // Track if background refresh failed; offer a retry action in UI + const [refreshFailed, setRefreshFailed] = useState(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; - - let cancelled = false; - const loadId = ++loadIdRef.current; - userEditedRef.current = false; - prevData.current = ""; - 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)(); + // Only attempt after initial model is ready to avoid racing the initial load + if (!modelPath) return; + // Avoid immediate double-load on open: the initial load has just completed. + if (skipNextRefreshRef.current) { + skipNextRefreshRef.current = false; + return; + } + // Only meaningful when a callable loader is provided (plain Promise cannot be "recalled") + 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)() + : (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; } - return dataSource ?? ""; - }); - - dataPromise - .then((data) => { - if (cancelled || loadId !== loadIdRef.current) return; - clearTimeout(timeoutId); - - if (userEditedRef.current) { - setCanSave(true); - return; - } - - const next = data ?? ""; + // 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); + } + // Ensure any previous error state is cleared after a successful refresh + if (isMountedRef.current && openRef.current) { + setRefreshFailed(null); + } + // If we previously failed to load, a successful refresh establishes a valid baseline + if (isMountedRef.current && openRef.current) { + setHasLoadedOnce(true); + } + } catch (err) { + // 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) { - if (editorRef.current) { - editorRef.current.setValue(next); - } else { - setInitialText(next); - } - } else { - setInitialText(next); - } - setCanSave(true); - }) - .catch((err) => { - if (cancelled || loadId !== loadIdRef.current) return; - clearTimeout(timeoutId); + const beforeMount = () => { + monacoInitialization(); + }; - if (!didTimeout) { - setInitialText(""); - } - if (!userEditedRef.current) { - setCanSave(false); - } - if (!didTimeout) { - showNotice.error(err); - } - }); + // 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)() + : (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 = () => { - try { - monacoInitialization(); - } catch (err) { - showNotice.error(err); - } - }; - - const onMount = (editor: monaco.editor.IStandaloneCodeEditor) => { + const onMount = async (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 = next; - userEditedRef.current = true; - if (!readOnly) { - setCanSave(true); + currData.current = value ?? editorRef.current?.getValue(); + onChange?.(prevData.current, currData.current); + // If the initial load failed, allow saving after the user makes an edit. + if (!hasLoadedOnce) { + setHasLoadedOnce(true); } - onChange?.(prevData.current, next); } catch (err) { showNotice.error(err); } - }; + }); const handleSave = useLockFn(async () => { try { - if (!readOnly && canSave) { - if (!editorRef.current) return; + // Disallow saving if initial content never loaded successfully to avoid accidental overwrite + 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 = (props: Props) => { } }); + // 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 = (props: Props) => { 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 = (props: Props) => { 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 = (props: Props) => { (props: Props) => { }} >
+ {/* Show overlay while loading or until modelPath is ready */} - {initialText !== null && ( + {/* Background refresh failure helper */} + {!!refreshFailed && ( +
+ + {t("shared.feedback.notifications.common.refreshFailed")} + + +
+ )} + {initialText !== null && modelPath && ( (props: Props) => { 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 = (props: Props) => { 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 = (props: Props) => { .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 = (props: Props) => {