From 9f34a27981257f547dc82cb4e0537c869f624d27 Mon Sep 17 00:00:00 2001 From: waleed Date: Mon, 27 Apr 2026 23:59:21 -0700 Subject: [PATCH] refactor(files): cleanup anti-patterns across file viewer components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six-pass cleanup over the file-viewer directory: Effects (you-might-not-need-an-effect): - AudioPreview, VideoPreview: replace reset useEffect with key={file.id} so the component remounts on file change — React's canonical solution - DocxPreview: same key-prop fix; removes a 5-setState reset effect that was also clearing containerRef.current.innerHTML unnecessarily Callbacks (you-might-not-need-a-callback): - handleEditorMount, handleEditorChange: remove useCallback — MonacoEditor is dynamic(), not React.memo, so reference stability has no observer - markSavedContent: remove useCallback — called only through an onSaveRef, never directly observed - DataTable.setInputRef: remove useCallback — callback refs on native elements are called regardless of reference identity Design tokens (emcn-design-review): - VideoPreview: bg-black → bg-[var(--surface-inverted)] - HtmlPreview iframe: bg-white → bg-[var(--surface-2)] useMemo, useState, and react-query passes found no issues. --- .../components/file-viewer/data-table.tsx | 6 +- .../components/file-viewer/file-viewer.tsx | 57 ++++++------------- .../components/file-viewer/preview-panel.tsx | 2 +- 3 files changed, 22 insertions(+), 43 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx index 421018ccb6..b9208d313e 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx @@ -1,6 +1,6 @@ 'use client' -import { forwardRef, memo, useCallback, useImperativeHandle, useRef, useState } from 'react' +import { forwardRef, memo, useImperativeHandle, useRef, useState } from 'react' import { cn } from '@/lib/core/utils/cn' interface EditConfig { @@ -49,12 +49,12 @@ const DataTableBase = forwardRef(function DataT [] ) - const setInputRef = useCallback((node: HTMLInputElement | null) => { + const setInputRef = (node: HTMLInputElement | null) => { if (node) { node.focus() node.select() } - }, []) + } const startEdit = (row: number, col: number, currentValue: string) => { if (!editConfig) return diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx index ca3c80c8a5..6a28769c38 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx @@ -521,9 +521,9 @@ function useTextEditorContentState(options: SyncTextEditorContentStateOptions) { dispatch({ type: 'edit', content }) }, []) - const markSavedContent = useCallback((content: string) => { + const markSavedContent = (content: string) => { dispatch({ type: 'save-success', content }) - }, []) + } return { content: state.content, @@ -596,15 +596,22 @@ export function FileViewer({ } if (category === 'audio-previewable') { - return + return } if (category === 'video-previewable') { - return + return } if (category === 'docx-previewable') { - return + return ( + + ) } if (category === 'pptx-previewable') { @@ -704,7 +711,6 @@ function TextEditor({ }) contentRef.current = content - // Sync external content (initial load + streaming) to Monaco model useEffect(() => { const editor = monacoEditorRef.current if (!editor) return @@ -713,9 +719,6 @@ function TextEditor({ const monacoValue = model.getValue() if (monacoValue === content) return - // Only override Monaco when we're pushing external content, not user edits: - // - During streaming/reconciling: always push - // - On first init (monacoValue matches last synced value): push if (isStreamInteractionLocked || monacoValue === lastSyncedContentRef.current) { model.setValue(content) lastSyncedContentRef.current = content @@ -825,7 +828,6 @@ function TextEditor({ const toggled = toggleMarkdownCheckbox(content, checkboxIndex, checked) if (toggled !== content) { setDraftContent(toggled) - // Also update Monaco synchronously so the user sees the change const model = monacoEditorRef.current?.getModel() if (model) { model.setValue(toggled) @@ -836,7 +838,7 @@ function TextEditor({ [content, setDraftContent] ) - const handleEditorMount: OnMount = useCallback((editor, monaco) => { + const handleEditorMount: OnMount = (editor, monaco) => { monacoEditorRef.current = editor editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => { @@ -854,14 +856,11 @@ function TextEditor({ hasAutoFocusedRef.current = true editor.focus() } - }, []) + } - const handleEditorChange = useCallback( - (value: string | undefined) => { - setDraftContent(value ?? '') - }, - [setDraftContent] - ) + const handleEditorChange = (value: string | undefined) => { + setDraftContent(value ?? '') + } const isStreaming = isStreamInteractionLocked const isEditorReadOnly = isStreamInteractionLocked || !canEdit @@ -1221,10 +1220,6 @@ const AudioPreview = memo(function AudioPreview({ } }, []) - useEffect(() => { - replaceBlobUrl(null) - }, [file.id, file.key, replaceBlobUrl]) - useEffect(() => { return () => { if (blobUrlRef.current) { @@ -1290,10 +1285,6 @@ const VideoPreview = memo(function VideoPreview({ } }, []) - useEffect(() => { - replaceBlobUrl(null) - }, [file.id, file.key, replaceBlobUrl]) - useEffect(() => { return () => { if (blobUrlRef.current) { @@ -1320,7 +1311,7 @@ const VideoPreview = memo(function VideoPreview({ } return ( -
+
{blobUrl && ( // biome-ignore lint/a11y/useMediaCaption: video from workspace files