Skip to content

Commit 4f1e44e

Browse files
waleedlatif1claude
andcommitted
fix(tables): address audit findings across table, undo hook, and store
- Add missing bounds check in handleCopy (c >= cols.length) matching handleCut for defensive consistency - Clear lastCheckboxRowRef in Ctrl+Space and Shift+Space to prevent stale shift-click checkbox range after keyboard selection - Fix stale snapshot race in patchRedoRowId/patchUndoRowId by reading state inside the set() updater instead of via get() outside it - Add metadata cleanup to create-column undo so column width is removed from both local state and server, symmetric with delete-column redo - Remove stale width key from columnWidths on column delete instead of persisting orphaned entries - Normalize undefined vs null in handleInlineSave change detection to prevent unnecessary mutations when oldValue is undefined - Use ghost.parentNode?.removeChild instead of document.body.removeChild in drag ghost cleanup to prevent throw on component unmount Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 659e02e commit 4f1e44e

File tree

3 files changed

+51
-28
lines changed

3 files changed

+51
-28
lines changed

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,7 @@ export function Table({
10251025
if (lastRow < 0) return
10261026
e.preventDefault()
10271027
setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
1028+
lastCheckboxRowRef.current = null
10281029
setSelectionAnchor({ rowIndex: 0, colIndex: a.colIndex })
10291030
setSelectionFocus({ rowIndex: lastRow, colIndex: a.colIndex })
10301031
setIsColumnSelection(true)
@@ -1038,6 +1039,7 @@ export function Table({
10381039
if (currentCols.length === 0) return
10391040
e.preventDefault()
10401041
setCheckedRows((prev) => (prev.size === 0 ? prev : EMPTY_CHECKED_ROWS))
1042+
lastCheckboxRowRef.current = null
10411043
setIsColumnSelection(false)
10421044
setSelectionAnchor({ rowIndex: a.rowIndex, colIndex: 0 })
10431045
setSelectionFocus({ rowIndex: a.rowIndex, colIndex: currentCols.length - 1 })
@@ -1354,6 +1356,7 @@ export function Table({
13541356
for (let r = sel.startRow; r <= sel.endRow; r++) {
13551357
const cells: string[] = []
13561358
for (let c = sel.startCol; c <= sel.endCol; c++) {
1359+
if (c >= cols.length) break
13571360
const row = pMap.get(r)
13581361
const value: unknown = row ? row.data[cols[c].name] : null
13591362
if (value === null || value === undefined) {
@@ -1593,15 +1596,16 @@ export function Table({
15931596
return
15941597
}
15951598

1596-
const oldValue = row.data[columnName]
1597-
const changed = !(oldValue === value) && !(oldValue === null && value === null)
1599+
const oldValue = row.data[columnName] ?? null
1600+
const normalizedValue = value ?? null
1601+
const changed = oldValue !== normalizedValue
15981602

15991603
if (changed) {
16001604
pushUndoRef.current({
16011605
type: 'update-cell',
16021606
rowId,
16031607
columnName,
1604-
previousValue: oldValue ?? null,
1608+
previousValue: oldValue,
16051609
newValue: value,
16061610
})
16071611
mutateRef.current({ rowId, data: { [columnName]: value } })
@@ -1806,13 +1810,19 @@ export function Table({
18061810
previousWidth,
18071811
})
18081812

1813+
const { [columnToDelete]: _removedWidth, ...cleanedWidths } = columnWidthsRef.current
1814+
setColumnWidths(cleanedWidths)
1815+
columnWidthsRef.current = cleanedWidths
1816+
18091817
if (currentOrder) {
18101818
currentOrder = currentOrder.filter((n) => n !== columnToDelete)
18111819
setColumnOrder(currentOrder)
18121820
updateMetadataRef.current({
1813-
columnWidths: columnWidthsRef.current,
1821+
columnWidths: cleanedWidths,
18141822
columnOrder: currentOrder,
18151823
})
1824+
} else {
1825+
updateMetadataRef.current({ columnWidths: cleanedWidths })
18161826
}
18171827

18181828
deleteNext(index + 1)
@@ -3147,7 +3157,7 @@ const ColumnHeaderMenu = React.memo(function ColumnHeaderMenu({
31473157
'position:absolute;top:-9999px;padding:4px 8px;background:var(--bg);border:1px solid var(--border);border-radius:4px;font-size:13px;font-weight:500;white-space:nowrap;color:var(--text-primary)'
31483158
document.body.appendChild(ghost)
31493159
e.dataTransfer.setDragImage(ghost, ghost.offsetWidth / 2, ghost.offsetHeight / 2)
3150-
requestAnimationFrame(() => document.body.removeChild(ghost))
3160+
requestAnimationFrame(() => ghost.parentNode?.removeChild(ghost))
31513161

31523162
onDragStart?.(column.name)
31533163
},

apps/sim/hooks/use-table-undo.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,16 @@ export function useTableUndo({
202202

203203
case 'create-column': {
204204
if (direction === 'undo') {
205-
deleteColumnMutation.mutate(action.columnName)
205+
deleteColumnMutation.mutate(action.columnName, {
206+
onSuccess: () => {
207+
const currentWidths = getColumnWidthsRef.current?.() ?? {}
208+
if (action.columnName in currentWidths) {
209+
const { [action.columnName]: _, ...rest } = currentWidths
210+
onColumnWidthsChangeRef.current?.(rest)
211+
updateMetadataMutation.mutate({ columnWidths: rest })
212+
}
213+
},
214+
})
206215
} else {
207216
addColumnMutation.mutate({
208217
name: action.columnName,

apps/sim/stores/table/store.ts

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -150,31 +150,35 @@ export const useTableUndoStore = create<TableUndoState>()(
150150
},
151151

152152
patchRedoRowId: (tableId: string, oldRowId: string, newRowId: string) => {
153-
const stacks = get().stacks[tableId]
154-
if (!stacks) return
155-
156-
const patchedRedo = stacks.redo.map((entry) => patchRowIdInEntry(entry, oldRowId, newRowId))
157-
158-
set((state) => ({
159-
stacks: {
160-
...state.stacks,
161-
[tableId]: { ...stacks, redo: patchedRedo },
162-
},
163-
}))
153+
set((state) => {
154+
const stacks = state.stacks[tableId]
155+
if (!stacks) return state
156+
const patchedRedo = stacks.redo.map((entry) =>
157+
patchRowIdInEntry(entry, oldRowId, newRowId)
158+
)
159+
return {
160+
stacks: {
161+
...state.stacks,
162+
[tableId]: { ...stacks, redo: patchedRedo },
163+
},
164+
}
165+
})
164166
},
165167

166168
patchUndoRowId: (tableId: string, oldRowId: string, newRowId: string) => {
167-
const stacks = get().stacks[tableId]
168-
if (!stacks) return
169-
170-
const patchedUndo = stacks.undo.map((entry) => patchRowIdInEntry(entry, oldRowId, newRowId))
171-
172-
set((state) => ({
173-
stacks: {
174-
...state.stacks,
175-
[tableId]: { ...stacks, undo: patchedUndo },
176-
},
177-
}))
169+
set((state) => {
170+
const stacks = state.stacks[tableId]
171+
if (!stacks) return state
172+
const patchedUndo = stacks.undo.map((entry) =>
173+
patchRowIdInEntry(entry, oldRowId, newRowId)
174+
)
175+
return {
176+
stacks: {
177+
...state.stacks,
178+
[tableId]: { ...stacks, undo: patchedUndo },
179+
},
180+
}
181+
})
178182
},
179183

180184
clear: (tableId: string) => {

0 commit comments

Comments
 (0)