Skip to content

Commit a05542a

Browse files
committed
fix(files): fix Monaco stale closure, XLSX Ctrl+S data loss, and async workbook mutation
Three bugs from Cursor Bugbot follow-up review: 1. Stale closure in handleEditorMount (Medium): useCallback([], []) captured content='' at first render. When Monaco mounts after content loads (e.g. switching from preview to editor mode), lastSyncedContentRef was never initialized and external content changes stopped syncing. Fixed by keeping a contentRef updated on every render and reading it inside handleEditorMount. 2. XLSX Ctrl+S discards active cell edit (Medium): handleSave read from workbookRef.current before DataTable's in-progress editValue was committed. Fixed by exposing commitEdit() from DataTable via useImperativeHandle (using an always-current editStateRef so the handle stays stable) and calling it at the top of handleSave. 3. Async workbook mutation fragility (Low): handleCellChange / handleHeaderChange updated the workbook inside import('xlsx').then(), creating microtask-order coupling with handleSave. Fixed by caching the xlsx module in xlsxModuleRef on first parse and using it synchronously in both handlers.
1 parent e794eeb commit a05542a

2 files changed

Lines changed: 60 additions & 20 deletions

File tree

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

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

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

66
interface EditConfig {
@@ -14,12 +14,41 @@ interface DataTableProps {
1414
editConfig?: EditConfig
1515
}
1616

17+
export interface DataTableHandle {
18+
commitEdit: () => void
19+
}
20+
1721
type EditingCell = { row: number; col: number } | null
1822

19-
export const DataTable = memo(function DataTable({ headers, rows, editConfig }: DataTableProps) {
23+
const DataTableBase = forwardRef<DataTableHandle, DataTableProps>(function DataTable(
24+
{ headers, rows, editConfig },
25+
ref
26+
) {
2027
const [editingCell, setEditingCell] = useState<EditingCell>(null)
2128
const [editValue, setEditValue] = useState('')
2229

30+
// Always-current ref so the imperative handle doesn't go stale
31+
const editStateRef = useRef({ editingCell, editValue, editConfig })
32+
editStateRef.current = { editingCell, editValue, editConfig }
33+
34+
useImperativeHandle(
35+
ref,
36+
() => ({
37+
commitEdit: () => {
38+
const { editingCell, editValue, editConfig } = editStateRef.current
39+
if (!editingCell || !editConfig) return
40+
const { row, col } = editingCell
41+
if (row === -1) {
42+
editConfig.onHeaderChange(col, editValue)
43+
} else {
44+
editConfig.onCellChange(row, col, editValue)
45+
}
46+
setEditingCell(null)
47+
},
48+
}),
49+
[]
50+
)
51+
2352
const setInputRef = useCallback((node: HTMLInputElement | null) => {
2453
if (node) {
2554
node.focus()
@@ -121,3 +150,5 @@ export const DataTable = memo(function DataTable({ headers, rows, editConfig }:
121150
</div>
122151
)
123152
})
153+
154+
export const DataTable = memo(DataTableBase)

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

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
useWorkspaceFileContent,
1818
} from '@/hooks/queries/workspace-files'
1919
import { useAutosave } from '@/hooks/use-autosave'
20+
import type { DataTableHandle } from './data-table'
2021
import { DataTable } from './data-table'
2122
import type { PdfDocumentSource } from './pdf-viewer'
2223
import { PreviewPanel, resolvePreviewType } from './preview-panel'
@@ -658,6 +659,7 @@ function TextEditor({
658659
const monacoEditorRef = useRef<Parameters<OnMount>[0] | null>(null)
659660
const lastSyncedContentRef = useRef('')
660661
const hasAutoFocusedRef = useRef(false)
662+
const contentRef = useRef('')
661663

662664
const [splitPct, setSplitPct] = useState(SPLIT_DEFAULT_PCT)
663665
const [isResizing, setIsResizing] = useState(false)
@@ -700,6 +702,7 @@ function TextEditor({
700702
streamingContent,
701703
streamingMode,
702704
})
705+
contentRef.current = content
703706

704707
// Sync external content (initial load + streaming) to Monaco model
705708
useEffect(() => {
@@ -841,9 +844,10 @@ function TextEditor({
841844
})
842845

843846
const model = editor.getModel()
844-
if (model && content && model.getValue() !== content) {
845-
model.setValue(content)
846-
lastSyncedContentRef.current = content
847+
const currentContent = contentRef.current
848+
if (model && currentContent && model.getValue() !== currentContent) {
849+
model.setValue(currentContent)
850+
lastSyncedContentRef.current = currentContent
847851
}
848852

849853
if (autoFocus && !hasAutoFocusedRef.current) {
@@ -1887,6 +1891,8 @@ const XlsxPreview = memo(function XlsxPreview({
18871891
const [isSaving, setIsSaving] = useState(false)
18881892
const isSavingRef = useRef(false)
18891893
const workbookRef = useRef<import('xlsx').WorkBook | null>(null)
1894+
const xlsxModuleRef = useRef<typeof import('xlsx') | null>(null)
1895+
const dataTableRef = useRef<DataTableHandle>(null)
18901896
const updateContent = useUpdateWorkspaceFileContent()
18911897
const updateContentRef = useRef(updateContent)
18921898
updateContentRef.current = updateContent
@@ -1904,6 +1910,7 @@ const XlsxPreview = memo(function XlsxPreview({
19041910
setRenderError(null)
19051911
setIsDirty(false)
19061912
const XLSX = await import('xlsx')
1913+
xlsxModuleRef.current = XLSX
19071914
const workbook = XLSX.read(new Uint8Array(data), { type: 'array' })
19081915
if (!cancelled) {
19091916
workbookRef.current = workbook
@@ -1966,17 +1973,16 @@ const XlsxPreview = memo(function XlsxPreview({
19661973
const handleCellChange = useCallback(
19671974
(row: number, col: number, value: string) => {
19681975
const wb = workbookRef.current
1969-
if (wb) {
1970-
// Update cell in workbook for save fidelity
1971-
import('xlsx').then(({ utils }) => {
1972-
const sheetName = sheetNames[activeSheet]
1973-
const ws = wb.Sheets[sheetName]
1974-
if (!ws) return
1975-
const cellAddr = utils.encode_cell({ r: row + 1, c: col })
1976+
const XLSX = xlsxModuleRef.current
1977+
if (wb && XLSX) {
1978+
const sheetName = sheetNames[activeSheet]
1979+
const ws = wb.Sheets[sheetName]
1980+
if (ws) {
1981+
const cellAddr = XLSX.utils.encode_cell({ r: row + 1, c: col })
19761982
const numValue = Number(value)
19771983
ws[cellAddr] =
19781984
value !== '' && !Number.isNaN(numValue) ? { t: 'n', v: numValue } : { t: 's', v: value }
1979-
})
1985+
}
19801986
}
19811987
setCurrentSheet((prev) => {
19821988
if (!prev) return prev
@@ -1993,14 +1999,14 @@ const XlsxPreview = memo(function XlsxPreview({
19931999
const handleHeaderChange = useCallback(
19942000
(col: number, value: string) => {
19952001
const wb = workbookRef.current
1996-
if (wb) {
1997-
import('xlsx').then(({ utils }) => {
1998-
const sheetName = sheetNames[activeSheet]
1999-
const ws = wb.Sheets[sheetName]
2000-
if (!ws) return
2001-
const cellAddr = utils.encode_cell({ r: 0, c: col })
2002+
const XLSX = xlsxModuleRef.current
2003+
if (wb && XLSX) {
2004+
const sheetName = sheetNames[activeSheet]
2005+
const ws = wb.Sheets[sheetName]
2006+
if (ws) {
2007+
const cellAddr = XLSX.utils.encode_cell({ r: 0, c: col })
20022008
ws[cellAddr] = { t: 's', v: value }
2003-
})
2009+
}
20042010
}
20052011
setCurrentSheet((prev) => {
20062012
if (!prev) return prev
@@ -2013,6 +2019,8 @@ const XlsxPreview = memo(function XlsxPreview({
20132019
)
20142020

20152021
const handleSave = useCallback(async () => {
2022+
// Commit any in-progress cell edit before reading the workbook
2023+
dataTableRef.current?.commitEdit()
20162024
const wb = workbookRef.current
20172025
if (!wb || isSavingRef.current) return
20182026

@@ -2116,6 +2124,7 @@ const XlsxPreview = memo(function XlsxPreview({
21162124
</div>
21172125
<div className='flex-1 overflow-auto p-6'>
21182126
<DataTable
2127+
ref={dataTableRef}
21192128
headers={currentSheet.headers}
21202129
rows={currentSheet.rows}
21212130
editConfig={editConfig}

0 commit comments

Comments
 (0)