Skip to content

Commit 9f34a27

Browse files
committed
refactor(files): cleanup anti-patterns across file viewer components
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.
1 parent a05542a commit 9f34a27

3 files changed

Lines changed: 22 additions & 43 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/data-table.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { forwardRef, memo, useCallback, useImperativeHandle, useRef, useState } from 'react'
3+
import { forwardRef, memo, useImperativeHandle, useRef, useState } from 'react'
44
import { cn } from '@/lib/core/utils/cn'
55

66
interface EditConfig {
@@ -49,12 +49,12 @@ const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataT
4949
[]
5050
)
5151

52-
const setInputRef = useCallback((node: HTMLInputElement | null) => {
52+
const setInputRef = (node: HTMLInputElement | null) => {
5353
if (node) {
5454
node.focus()
5555
node.select()
5656
}
57-
}, [])
57+
}
5858

5959
const startEdit = (row: number, col: number, currentValue: string) => {
6060
if (!editConfig) return

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,9 @@ function useTextEditorContentState(options: SyncTextEditorContentStateOptions) {
521521
dispatch({ type: 'edit', content })
522522
}, [])
523523

524-
const markSavedContent = useCallback((content: string) => {
524+
const markSavedContent = (content: string) => {
525525
dispatch({ type: 'save-success', content })
526-
}, [])
526+
}
527527

528528
return {
529529
content: state.content,
@@ -596,15 +596,22 @@ export function FileViewer({
596596
}
597597

598598
if (category === 'audio-previewable') {
599-
return <AudioPreview file={file} workspaceId={workspaceId} />
599+
return <AudioPreview key={file.id} file={file} workspaceId={workspaceId} />
600600
}
601601

602602
if (category === 'video-previewable') {
603-
return <VideoPreview file={file} workspaceId={workspaceId} />
603+
return <VideoPreview key={file.id} file={file} workspaceId={workspaceId} />
604604
}
605605

606606
if (category === 'docx-previewable') {
607-
return <DocxPreview file={file} workspaceId={workspaceId} streamingContent={streamingContent} />
607+
return (
608+
<DocxPreview
609+
key={file.id}
610+
file={file}
611+
workspaceId={workspaceId}
612+
streamingContent={streamingContent}
613+
/>
614+
)
608615
}
609616

610617
if (category === 'pptx-previewable') {
@@ -704,7 +711,6 @@ function TextEditor({
704711
})
705712
contentRef.current = content
706713

707-
// Sync external content (initial load + streaming) to Monaco model
708714
useEffect(() => {
709715
const editor = monacoEditorRef.current
710716
if (!editor) return
@@ -713,9 +719,6 @@ function TextEditor({
713719
const monacoValue = model.getValue()
714720
if (monacoValue === content) return
715721

716-
// Only override Monaco when we're pushing external content, not user edits:
717-
// - During streaming/reconciling: always push
718-
// - On first init (monacoValue matches last synced value): push
719722
if (isStreamInteractionLocked || monacoValue === lastSyncedContentRef.current) {
720723
model.setValue(content)
721724
lastSyncedContentRef.current = content
@@ -825,7 +828,6 @@ function TextEditor({
825828
const toggled = toggleMarkdownCheckbox(content, checkboxIndex, checked)
826829
if (toggled !== content) {
827830
setDraftContent(toggled)
828-
// Also update Monaco synchronously so the user sees the change
829831
const model = monacoEditorRef.current?.getModel()
830832
if (model) {
831833
model.setValue(toggled)
@@ -836,7 +838,7 @@ function TextEditor({
836838
[content, setDraftContent]
837839
)
838840

839-
const handleEditorMount: OnMount = useCallback((editor, monaco) => {
841+
const handleEditorMount: OnMount = (editor, monaco) => {
840842
monacoEditorRef.current = editor
841843

842844
editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => {
@@ -854,14 +856,11 @@ function TextEditor({
854856
hasAutoFocusedRef.current = true
855857
editor.focus()
856858
}
857-
}, [])
859+
}
858860

859-
const handleEditorChange = useCallback(
860-
(value: string | undefined) => {
861-
setDraftContent(value ?? '')
862-
},
863-
[setDraftContent]
864-
)
861+
const handleEditorChange = (value: string | undefined) => {
862+
setDraftContent(value ?? '')
863+
}
865864

866865
const isStreaming = isStreamInteractionLocked
867866
const isEditorReadOnly = isStreamInteractionLocked || !canEdit
@@ -1221,10 +1220,6 @@ const AudioPreview = memo(function AudioPreview({
12211220
}
12221221
}, [])
12231222

1224-
useEffect(() => {
1225-
replaceBlobUrl(null)
1226-
}, [file.id, file.key, replaceBlobUrl])
1227-
12281223
useEffect(() => {
12291224
return () => {
12301225
if (blobUrlRef.current) {
@@ -1290,10 +1285,6 @@ const VideoPreview = memo(function VideoPreview({
12901285
}
12911286
}, [])
12921287

1293-
useEffect(() => {
1294-
replaceBlobUrl(null)
1295-
}, [file.id, file.key, replaceBlobUrl])
1296-
12971288
useEffect(() => {
12981289
return () => {
12991290
if (blobUrlRef.current) {
@@ -1320,7 +1311,7 @@ const VideoPreview = memo(function VideoPreview({
13201311
}
13211312

13221313
return (
1323-
<div className='flex h-full items-center justify-center bg-black'>
1314+
<div className='flex h-full items-center justify-center bg-[var(--surface-inverted)]'>
13241315
{blobUrl && (
13251316
// biome-ignore lint/a11y/useMediaCaption: video from workspace files
13261317
<video src={blobUrl} controls className='max-h-full max-w-full' />
@@ -1440,16 +1431,6 @@ const DocxPreview = memo(function DocxPreview({
14401431
const [rendering, setRendering] = useState(false)
14411432
const [hasRenderedPreview, setHasRenderedPreview] = useState(false)
14421433

1443-
useEffect(() => {
1444-
lastSuccessfulHtmlRef.current = ''
1445-
setRenderError(null)
1446-
setRendering(false)
1447-
setHasRenderedPreview(false)
1448-
if (containerRef.current) {
1449-
containerRef.current.innerHTML = ''
1450-
}
1451-
}, [file.id, file.key])
1452-
14531434
useEffect(() => {
14541435
if (!containerRef.current || !fileData || streamingContent !== undefined) return
14551436

@@ -2019,7 +2000,6 @@ const XlsxPreview = memo(function XlsxPreview({
20192000
)
20202001

20212002
const handleSave = useCallback(async () => {
2022-
// Commit any in-progress cell edit before reading the workbook
20232003
dataTableRef.current?.commitEdit()
20242004
const wb = workbookRef.current
20252005
if (!wb || isSavingRef.current) return
@@ -2033,7 +2013,6 @@ const XlsxPreview = memo(function XlsxPreview({
20332013
const binary: number[] = XLSX.write(wb, { type: 'array', bookType: 'xlsx' })
20342014
const bytes = new Uint8Array(binary)
20352015

2036-
// Convert to base64 in chunks to avoid call stack overflow
20372016
const chunkSize = 8192
20382017
const parts: string[] = []
20392018
for (let i = 0; i < bytes.length; i += chunkSize) {

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-panel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ const HtmlPreview = memo(function HtmlPreview({ content }: { content: string })
851851
sandbox='allow-scripts'
852852
referrerPolicy='no-referrer'
853853
title='HTML Preview'
854-
className='h-full w-full border-0 bg-white'
854+
className='h-full w-full border-0 bg-[var(--surface-2)]'
855855
/>
856856
)}
857857
</div>

0 commit comments

Comments
 (0)