From 66347d82eba8cb88630f099fb1d94b5d21b66909 Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Thu, 28 Mar 2024 07:09:18 +0000 Subject: [PATCH 1/6] Add function to deeply compare arrays --- src/util.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/util.ts b/src/util.ts index fa8c18c7..353e38b3 100644 --- a/src/util.ts +++ b/src/util.ts @@ -34,6 +34,17 @@ export function range(end: number, start = 0, step = 1): number[] { return array; } +/** Perform deep comparison of two (nested) arrays. */ +export function deepArrayComparison(arr1: any[], arr2: any[]): boolean { + if (arr1.length !== arr2.length) return false; + return arr1.every((element, index) => { + if (Array.isArray(element) && Array.isArray(arr2[index])) { + return deepArrayComparison(element, arr2[index]); + } + return element === arr2[index]; + }); +} + /** Return whether given point is active */ export function isActive( active: Types.StoreState["active"], From fcf2a53be42dbb6bc68bc168de1444fd53c4713f Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Thu, 28 Mar 2024 07:10:18 +0000 Subject: [PATCH 2/6] Fix bug (infinite render loop) caused by wrong dependence arrays --- src/Spreadsheet.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Spreadsheet.tsx b/src/Spreadsheet.tsx index 70d0b3a9..9b203cb3 100644 --- a/src/Spreadsheet.tsx +++ b/src/Spreadsheet.tsx @@ -16,6 +16,7 @@ import { getCSV, shouldHandleClipboardEvent, isFocusedWithin, + deepArrayComparison, } from "./util"; import DefaultTable from "./Table"; @@ -235,13 +236,12 @@ const Spreadsheet = ( // Listen to data changes const prevDataRef = React.useRef>(state.model.data); React.useEffect(() => { - if (state.model.data !== prevDataRef.current) { + if (!deepArrayComparison(state.model.data, prevDataRef.current)) { // Call on change only if the data change internal - if (state.model.data !== props.data) { + if (!deepArrayComparison(state.model.data, props.data)) { onChange(state.model.data); } } - prevDataRef.current = state.model.data; }, [state.model.data, onChange, props.data]); @@ -252,7 +252,6 @@ const Spreadsheet = ( if (state?.model?.evaluatedData !== prevEvaluatedDataRef?.current) { onEvaluatedDataChange(state?.model?.evaluatedData); } - prevEvaluatedDataRef.current = state.model.evaluatedData; }, [state?.model?.evaluatedData, onEvaluatedDataChange]); From 4bab63789b2fe12e300e23faf0f6ea67f6c51a8e Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Mon, 15 Apr 2024 07:16:49 +0000 Subject: [PATCH 3/6] Improve performance by using a new primitive variable to trigger events --- src/Spreadsheet.tsx | 19 +++++++++---------- src/reducer.ts | 5 +++++ src/types.ts | 1 + src/util.test.ts | 1 + src/util.ts | 11 ----------- 5 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/Spreadsheet.tsx b/src/Spreadsheet.tsx index 9b203cb3..8385c2b2 100644 --- a/src/Spreadsheet.tsx +++ b/src/Spreadsheet.tsx @@ -16,7 +16,6 @@ import { getCSV, shouldHandleClipboardEvent, isFocusedWithin, - deepArrayComparison, } from "./util"; import DefaultTable from "./Table"; @@ -234,16 +233,16 @@ const Spreadsheet = ( }, [onActivate, onBlur, state.active]); // Listen to data changes - const prevDataRef = React.useRef>(state.model.data); + const currentModelDataRef = React.useRef>( + state.model.data + ); React.useEffect(() => { - if (!deepArrayComparison(state.model.data, prevDataRef.current)) { - // Call on change only if the data change internal - if (!deepArrayComparison(state.model.data, props.data)) { - onChange(state.model.data); - } - } - prevDataRef.current = state.model.data; - }, [state.model.data, onChange, props.data]); + currentModelDataRef.current = state.model.data; + }, [state.model.data]); + + React.useEffect(() => { + onChange(currentModelDataRef.current); + }, [state.lastUpdateDate, onChange]); const prevEvaluatedDataRef = React.useRef>( state.model.evaluatedData diff --git a/src/reducer.ts b/src/reducer.ts index b9cc59c2..437b6521 100644 --- a/src/reducer.ts +++ b/src/reducer.ts @@ -20,6 +20,7 @@ export const INITIAL_STATE: Types.StoreState = { rowDimensions: {}, columnDimensions: {}, lastChanged: null, + lastUpdateDate: null, hasPasted: false, cut: false, dragging: false, @@ -132,6 +133,7 @@ export default function reducer( ...state, model: updateCellValue(state.model, active, cellData), lastChanged: active, + lastUpdateDate: new Date(), }; } case Actions.SET_CELL_DIMENSIONS: { @@ -206,6 +208,7 @@ export default function reducer( hasPasted: true, mode: "view", lastCommit: commit, + lastUpdateDate: new Date(), }; } @@ -276,6 +279,7 @@ export default function reducer( hasPasted: true, mode: "view", lastCommit: acc.commit, + lastUpdateDate: new Date(), }; } @@ -385,6 +389,7 @@ function clear(state: Types.StoreState): Types.StoreState { ...state, model: new Model(createFormulaParser, newData), ...commit(changes), + lastUpdateDate: new Date(), }; } diff --git a/src/types.ts b/src/types.ts index fa47a46a..0b0a1634 100644 --- a/src/types.ts +++ b/src/types.ts @@ -60,6 +60,7 @@ export type StoreState = { >; dragging: boolean; lastChanged: Point | null; + lastUpdateDate: Date | null; lastCommit: null | CellChange[]; }; diff --git a/src/util.test.ts b/src/util.test.ts index 47fb6416..f7ca5ae2 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -54,6 +54,7 @@ const EXAMPLE_STATE: Types.StoreState = { }, }, lastChanged: null, + lastUpdateDate: null, hasPasted: false, cut: false, dragging: false, diff --git a/src/util.ts b/src/util.ts index 353e38b3..fa8c18c7 100644 --- a/src/util.ts +++ b/src/util.ts @@ -34,17 +34,6 @@ export function range(end: number, start = 0, step = 1): number[] { return array; } -/** Perform deep comparison of two (nested) arrays. */ -export function deepArrayComparison(arr1: any[], arr2: any[]): boolean { - if (arr1.length !== arr2.length) return false; - return arr1.every((element, index) => { - if (Array.isArray(element) && Array.isArray(arr2[index])) { - return deepArrayComparison(element, arr2[index]); - } - return element === arr2[index]; - }); -} - /** Return whether given point is active */ export function isActive( active: Types.StoreState["active"], From 845b5c469861eb28382e4ab0c554f59735235170 Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Mon, 15 Apr 2024 10:00:08 +0000 Subject: [PATCH 4/6] Update test to handle date and milliseconds for the setCellData action --- src/reducer.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/reducer.test.ts b/src/reducer.test.ts index 1963e08e..501f6ea9 100644 --- a/src/reducer.test.ts +++ b/src/reducer.test.ts @@ -223,7 +223,13 @@ describe("reducer", () => { ], ]; test.each(cases)("%s", (name, state, action, expected) => { - expect(reducer(state, action)).toEqual(expected); + if (name === "setCellData") { + // Addressing this case separately since the test may fail due to slight time-related + // differences between the generation of the expected and obtained results. + const result = reducer(state, action); + result.lastUpdateDate = null; + expect(result).toEqual(expected); + } else expect(reducer(state, action)).toEqual(expected); }); }); From 6432922ef8594fac4766e31b477ed6b5f26f098e Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Mon, 22 Apr 2024 08:14:40 +0000 Subject: [PATCH 5/6] Remove redundant renderings --- src/Cell.tsx | 9 +++++++-- src/Spreadsheet.tsx | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Cell.tsx b/src/Cell.tsx index 7bf79e08..290a7280 100644 --- a/src/Cell.tsx +++ b/src/Cell.tsx @@ -57,15 +57,20 @@ export const Cell: React.FC = ({ [setCellDimensions, select, dragging, point] ); + const modeRef = React.useRef(); + React.useEffect(() => { + modeRef.current = mode; + }, [mode]); + React.useEffect(() => { const root = rootRef.current; if (selected && root) { setCellDimensions(point, getOffsetRect(root)); } - if (root && active && mode === "view") { + if (root && active && modeRef.current === "view") { root.focus(); } - }, [setCellDimensions, selected, active, mode, point, data]); + }, [setCellDimensions, selected, active, point, data]); if (data && data.DataViewer) { // @ts-ignore diff --git a/src/Spreadsheet.tsx b/src/Spreadsheet.tsx index 8385c2b2..d94d58e6 100644 --- a/src/Spreadsheet.tsx +++ b/src/Spreadsheet.tsx @@ -171,7 +171,7 @@ const Spreadsheet = ( const size = React.useMemo(() => { return calculateSpreadsheetSize(state.model.data, rowLabels, columnLabels); - }, [state.model.data, rowLabels, columnLabels]); + }, [state.lastUpdateDate, rowLabels, columnLabels]); const mode = state.mode; @@ -241,6 +241,7 @@ const Spreadsheet = ( }, [state.model.data]); React.useEffect(() => { + if (state.lastUpdateDate === null) return; onChange(currentModelDataRef.current); }, [state.lastUpdateDate, onChange]); From 5a24c814126a530e61150a228dc1ae2cf48990fb Mon Sep 17 00:00:00 2001 From: Edoardo Gruppi Date: Mon, 22 Apr 2024 08:28:35 +0000 Subject: [PATCH 6/6] Fix rendering issue: table size --- src/Spreadsheet.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spreadsheet.tsx b/src/Spreadsheet.tsx index d94d58e6..3d0dc79c 100644 --- a/src/Spreadsheet.tsx +++ b/src/Spreadsheet.tsx @@ -171,7 +171,7 @@ const Spreadsheet = ( const size = React.useMemo(() => { return calculateSpreadsheetSize(state.model.data, rowLabels, columnLabels); - }, [state.lastUpdateDate, rowLabels, columnLabels]); + }, [state.model.data, rowLabels, columnLabels]); const mode = state.mode;