From c84bb91f4ad48c7a83d6e0ceb9370637b149c5ea Mon Sep 17 00:00:00 2001 From: Sline Date: Sun, 21 Dec 2025 17:10:52 +0800 Subject: [PATCH] refactor(editor-viewer): simplify loading/save flow with timeout fallback (#5905) --- src/components/profile/editor-viewer.tsx | 377 +++++++---------------- 1 file changed, 115 insertions(+), 262 deletions(-) diff --git a/src/components/profile/editor-viewer.tsx b/src/components/profile/editor-viewer.tsx index 99d645121..e1ae9b723 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, useRef, useState } from "react"; +import { ReactNode, useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import pac from "types-pac/pac.d.ts?raw"; @@ -35,10 +35,8 @@ 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; reloads when this or language changes. + // Logical document id; used to build a stable model path. dataKey?: string | number; readOnly?: boolean; language: T; @@ -47,16 +45,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; @@ -81,237 +79,139 @@ export const EditorViewer = (props: Props) => { const resolvedTitle = title ?? t("profiles.components.menu.editFile"); - const editorRef = useRef(undefined); - const prevData = useRef(""); - const currData = useRef(""); - // Hold the latest loader without making effects depend on its identity + const editorRef = useRef(null); + const prevData = useRef(""); + const currData = useRef(""); + const userEditedRef = useRef(false); + const loadIdRef = useRef(0); const initialDataRef = useRef["initialData"]>(initialData); - // 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 [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 + + 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; + 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 - 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; + + 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 update when untouched and value changed - const userUntouched = currData.current === prevData.current; - if (userUntouched && next !== prevData.current) { + return dataSource ?? ""; + }); + + dataPromise + .then((data) => { + if (cancelled || loadId !== loadIdRef.current) return; + clearTimeout(timeoutId); + + if (userEditedRef.current) { + setCanSave(true); + return; + } + + const next = data ?? ""; 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, - ); - } - } - }); - const beforeMount = () => { - monacoInitialization(); - }; + 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); - // 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); - } - })(); + if (!didTimeout) { + setInitialText(""); + } + if (!userEditedRef.current) { + setCanSave(false); + } + if (!didTimeout) { + showNotice.error(err); + } + }); return () => { cancelled = true; + clearTimeout(timeoutId); }; }, [open, dataKey, language]); - /* eslint-enable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */ - 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 = useLockFn(async (value?: string) => { + const beforeMount = () => { try { - 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); - } + monacoInitialization(); } catch (err) { showNotice.error(err); } - }); + }; + + const onMount = (editor: monaco.editor.IStandaloneCodeEditor) => { + editorRef.current = editor; + }; + + const handleChange = (value?: string) => { + try { + const next = value ?? editorRef.current?.getValue() ?? ""; + currData.current = next; + userEditedRef.current = true; + if (!readOnly) { + setCanSave(true); + } + onChange?.(prevData.current, next); + } catch (err) { + showNotice.error(err); + } + }; const handleSave = useLockFn(async () => { try { - // 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; - } + if (!readOnly && canSave) { + 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; } } @@ -321,17 +221,15 @@ 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 || effectiveReadOnly) return; + if (!editorRef.current || readOnly) 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", @@ -361,8 +259,6 @@ 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 {} @@ -371,13 +267,10 @@ 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(); - modelChangeDisposableRef.current?.dispose(); - modelChangeDisposableRef.current = null; - editorRef.current = undefined; + editorRef.current = null; }; }, []); @@ -394,7 +287,6 @@ export const EditorViewer = (props: Props) => { (props: Props) => { }} >
- {/* Show overlay while loading or until modelPath is ready */} - {/* Background refresh failure helper */} - {!!refreshFailed && ( -
- - {t("shared.feedback.notifications.common.refreshFailed")} - - -
- )} - {initialText !== null && modelPath && ( + {initialText !== null && ( (props: Props) => { enabled: document.documentElement.clientWidth >= 1500, }, mouseWheelZoom: true, - readOnly: effectiveReadOnly, + readOnly, readOnlyMessage: { value: t("profiles.modals.editor.messages.readOnly"), }, @@ -468,7 +322,7 @@ export const EditorViewer = (props: Props) => { other: true, }, padding: { - top: 33, // Top padding to prevent snippet overlap + top: 33, }, fontFamily: `Fira Code, JetBrains Mono, Roboto Mono, "Source Code Pro", Consolas, Menlo, Monaco, monospace, "Courier New", "Apple Color Emoji"${ getSystem() === "windows" ? ", twemoji mozilla" : "" @@ -526,7 +380,6 @@ 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 {} @@ -546,7 +399,7 @@ export const EditorViewer = (props: Props) => {