fix: preserve cell identity in commit callbacks

src/ActiveCell.tsx

@@ -46,6 +46,7 @@ const ActiveCell: React.FC<Props> = (props) => {
   );
 
   const initialCellRef = React.useRef<Types.CellBase | undefined>(undefined);
+  const prevModeRef = React.useRef<Types.Mode>("view");
   const prevActiveRef = React.useRef<Point.Point | null>(null);
   const prevCellRef = React.useRef<Types.CellBase | undefined>(undefined);
 
@@ -69,37 +70,32 @@ const ActiveCell: React.FC<Props> = (props) => {
   React.useEffect(() => {
     const prevActive = prevActiveRef.current;
     const prevCell = prevCellRef.current;
-    prevActiveRef.current = active;
-    prevCellRef.current = cell;
-
-    if (!prevActive || !prevCell) {
-      return;
-    }
-
-    // Commit
+    const prevMode = prevModeRef.current;
     const coordsChanged =
-      active?.row !== prevActive.row || active?.column !== prevActive.column;
-    const exitedEditMode = mode !== "edit";
+      active?.row !== prevActive?.row || active?.column !== prevActive?.column;
 
-    if (coordsChanged || exitedEditMode) {
+    // An edit belongs to the cell where it started, even after navigation.
+    if (prevMode === "edit" && (coordsChanged || mode !== "edit")) {
       const initialCell = initialCellRef.current;
-      if (prevCell !== initialCell) {
+      const nextCell = coordsChanged ? prevCell : cell;
+      if (prevActive && nextCell !== initialCell) {
         commit([
           {
             prevCell: initialCell || null,
-            nextCell: prevCell,
-          },
-        ]);
-      } else if (!coordsChanged && cell !== prevCell) {
-        commit([
-          {
-            prevCell,
-            nextCell: cell || null,
+            nextCell: nextCell || null,
+            point: prevActive,
           },
         ]);
       }
-      initialCellRef.current = cell;
     }
+
+    if (mode === "edit" && (prevMode !== "edit" || coordsChanged)) {
+      // Typing to replace clears the value on entry; retain the pre-edit cell.
+      initialCellRef.current = coordsChanged ? cell : prevCell;
+    }
+    prevActiveRef.current = active;
+    prevCellRef.current = cell;
+    prevModeRef.current = mode;
   });
 
   const DataEditor = (cell && cell.DataEditor) || props.DataEditor;

src/Spreadsheet.test.tsx

@@ -579,3 +579,133 @@ function getHTMLCollectionIndexOf(
 function getSpreadsheetElement(): Element {
   return safeQuerySelector(document, ".Spreadsheet");
 }
+
+describe("cell commit identity", () => {
+  test("clearing a different cell reports the cleared cell, not the last edit", () => {
+    const onCellCommit = jest.fn();
+    const data = [[{ value: "A" }, { value: "B" }]];
+    const { container } = render(
+      <Spreadsheet data={data} onCellCommit={onCellCommit} />
+    );
+    const cells = container.querySelectorAll("td");
+    fireEvent.mouseDown(cells[1]);
+    fireEvent.keyDown(
+      safeQuerySelector(container, ".Spreadsheet__active-cell"),
+      { key: "Enter" }
+    );
+    fireEvent.change(safeQuerySelector(container, "input"), {
+      target: { value: "edited B" },
+    });
+    fireEvent.keyDown(safeQuerySelector(container, "input"), { key: "Enter" });
+    fireEvent.mouseDown(cells[0]);
+    onCellCommit.mockClear();
+    fireEvent.keyDown(
+      safeQuerySelector(container, ".Spreadsheet__active-cell"),
+      { key: "Delete" }
+    );
+    expect(onCellCommit).toHaveBeenCalledTimes(1);
+    expect(onCellCommit).toHaveBeenCalledWith(
+      data[0][0],
+      { value: undefined },
+      { row: 0, column: 0 }
+    );
+  });
+
+  test("entering and leaving an unchanged cell does not commit; editing retains its old value", () => {
+    const onCellCommit = jest.fn();
+    const data = [[{ value: "original" }]];
+    const { container } = render(
+      <Spreadsheet data={data} onCellCommit={onCellCommit} />
+    );
+    fireEvent.mouseDown(safeQuerySelector(container, "td"));
+    expect(onCellCommit).not.toHaveBeenCalled();
+    fireEvent.keyDown(
+      safeQuerySelector(container, ".Spreadsheet__active-cell"),
+      { key: "Enter" }
+    );
+    fireEvent.keyDown(safeQuerySelector(container, "input"), { key: "Enter" });
+    expect(onCellCommit).not.toHaveBeenCalled();
+    fireEvent.keyDown(
+      safeQuerySelector(container, ".Spreadsheet__active-cell"),
+      { key: "Enter" }
+    );
+    fireEvent.change(safeQuerySelector(container, "input"), {
+      target: { value: "changed" },
+    });
+    fireEvent.keyDown(safeQuerySelector(container, "input"), { key: "Enter" });
+    expect(onCellCommit).toHaveBeenCalledTimes(1);
+    expect(onCellCommit).toHaveBeenCalledWith(
+      data[0][0],
+      { value: "changed" },
+      { row: 0, column: 0 }
+    );
+  });
+
+  test("changing the callback does not replay a previously delivered commit", () => {
+    const first = jest.fn();
+    const second = jest.fn();
+    const data = [[{ value: "A" }]];
+    const { container, rerender } = render(
+      <Spreadsheet data={data} onCellCommit={first} />
+    );
+    fireEvent.mouseDown(safeQuerySelector(container, "td"));
+    fireEvent.keyDown(
+      safeQuerySelector(container, ".Spreadsheet__active-cell"),
+      { key: "Delete" }
+    );
+    first.mockClear();
+    rerender(<Spreadsheet data={data} onCellCommit={second} />);
+    expect(second).not.toHaveBeenCalled();
+  });
+});
+
+test("pasting a row reports each destination coordinate exactly once", () => {
+  const onCellCommit = jest.fn();
+  const data = [[{ value: "A" }, { value: "B" }]];
+  const { container } = render(
+    <Spreadsheet data={data} onCellCommit={onCellCommit} />
+  );
+  fireEvent.mouseDown(safeQuerySelector(container, "td"));
+  fireEvent.paste(safeQuerySelector(container, ".Spreadsheet__active-cell"), {
+    clipboardData: { getData: () => "X\tY" },
+  });
+  expect(onCellCommit).toHaveBeenCalledTimes(2);
+  expect(onCellCommit).toHaveBeenNthCalledWith(
+    1,
+    data[0][0],
+    { value: "X" },
+    { row: 0, column: 0 }
+  );
+  expect(onCellCommit).toHaveBeenNthCalledWith(
+    2,
+    data[0][1],
+    { value: "Y" },
+    { row: 0, column: 1 }
+  );
+});
+
+test("cut and paste commits the source removal and destination separately", () => {
+  const onCellCommit = jest.fn();
+  const data = [[{ value: "A" }, { value: "B" }]];
+  const { container } = render(
+    <Spreadsheet data={data} onCellCommit={onCellCommit} />
+  );
+  const cells = container.querySelectorAll("td");
+  fireEvent.mouseDown(cells[0]);
+  fireEvent.cut(safeQuerySelector(container, ".Spreadsheet__active-cell"), {
+    clipboardData: { setData: jest.fn() },
+  });
+  fireEvent.mouseDown(cells[1]);
+  fireEvent.paste(safeQuerySelector(container, ".Spreadsheet__active-cell"), {
+    clipboardData: { getData: () => "A" },
+  });
+  expect(onCellCommit).toHaveBeenCalledTimes(2);
+  expect(onCellCommit).toHaveBeenNthCalledWith(1, data[0][0], null, {
+    row: 0,
+    column: 0,
+  });
+  expect(onCellCommit).toHaveBeenNthCalledWith(2, data[0][1], data[0][0], {
+    row: 0,
+    column: 1,
+  });
+});

src/Spreadsheet.tsx

@@ -295,13 +295,18 @@ const Spreadsheet = <SpreadsheetRef, CellType extends Types.CellBase>(
   }, [state.mode, onModeChange]);
 
   // Listen to last commit changes
-  const prevLastCommitRef = React.useRef<null | Types.CellChange[]>(
+  const prevLastCommitRef = React.useRef<Types.StoreState["lastCommit"]>(
     state.lastCommit
   );
   React.useEffect(() => {
     if (state.lastCommit && state.lastCommit !== prevLastCommitRef.current) {
+      prevLastCommitRef.current = state.lastCommit;
       for (const change of state.lastCommit) {
-        onCellCommit(change.prevCell, change.nextCell, state.lastChanged);
+        onCellCommit(
+          change.prevCell,
+          change.nextCell,
+          change.point ?? state.lastChanged
+        );
       }
     }
   }, [onCellCommit, state.lastChanged, state.lastCommit]);

src/reducer.ts

@@ -189,9 +189,17 @@ export default function reducer(
             ? Matrix.unset(state.copied.start, state.model.data)
             : state.model.data;
         const commit: Types.StoreState["lastCommit"] = [];
+        if (state.cut && state.copied) {
+          commit.push({
+            point: state.copied.start,
+            prevCell: Matrix.get(state.copied.start, state.model.data) || null,
+            nextCell: null,
+          });
+        }
         for (const point of selectedRange || []) {
           const currentCell = Matrix.get(point, state.model.data);
           commit.push({
+            point,
             prevCell: currentCell || null,
             nextCell: cell || null,
           });
@@ -234,10 +242,16 @@ export default function reducer(
               row: point.row + state.copied.start.row,
               column: point.column + state.copied.start.column,
             };
+            commit = [
+              ...commit,
+              {
+                point: prevPoint,
+                prevCell: Matrix.get(prevPoint, acc.data) || null,
+                nextCell: null,
+              },
+            ];
             nextData = Matrix.unset(prevPoint, acc.data);
           }
-
-          commit = [...commit, { prevCell: cell || null, nextCell: null }];
         }
 
         if (!Matrix.has(nextPoint, paddedData)) {
@@ -249,6 +263,7 @@ export default function reducer(
         commit = [
           ...commit,
           {
+            point: nextPoint,
             prevCell: currentCell,
             nextCell: cell || null,
           },
@@ -303,7 +318,7 @@ export default function reducer(
       if (state.mode === "view" && state.active) {
         const selectedRange = state.selected.toRange(state.model.data);
         if (selectedRange?.size() === 1) {
-          return edit(clear(state));
+          return edit(clear(state, false));
         }
         return edit(state);
       }
@@ -356,7 +371,7 @@ function edit(state: Types.StoreState): Types.StoreState {
   return { ...state, mode: "edit" };
 }
 
-function clear(state: Types.StoreState): Types.StoreState {
+function clear(state: Types.StoreState, notify = true): Types.StoreState {
   if (!state.active) {
     return state;
   }
@@ -379,6 +394,7 @@ function clear(state: Types.StoreState): Types.StoreState {
     const cell = Matrix.get(point, state.model.data);
     const clearedCell = clearCell(cell);
     changes.push({
+      point,
       prevCell: cell || null,
       nextCell: clearedCell || null,
     });
@@ -388,7 +404,7 @@ function clear(state: Types.StoreState): Types.StoreState {
   return {
     ...state,
     model: new Model(createFormulaParser, newData),
-    ...commit(changes),
+    ...(notify ? commit(changes) : {}),
   };
 }
 
@@ -448,8 +464,8 @@ const keyDownHandlers: KeyDownHandlers = {
   ArrowRight: go(0, +1),
   Tab: go(0, +1),
   Enter: edit,
-  Backspace: clear,
-  Delete: clear,
+  Backspace: (state) => clear(state),
+  Delete: (state) => clear(state),
   Escape: blur,
 };
 

src/types.ts

@@ -60,7 +60,7 @@ export type StoreState<Cell extends CellBase = CellBase> = {
   >;
   dragging: boolean;
   lastChanged: Point | null;
-  lastCommit: null | CellChange<Cell>[];
+  lastCommit: null | CommitChanges<Cell>;
 };
 
 export type CellChange<Cell extends CellBase = CellBase> = {
@@ -203,9 +203,8 @@ export type CornerIndicatorProps = {
 export type CornerIndicatorComponent =
   React.ComponentType<CornerIndicatorProps>;
 
-export type CommitChanges<Cell extends CellBase = CellBase> = Array<{
-  prevCell: Cell | null;
-  nextCell: Cell | null;
-}>;
+export type CommitChanges<Cell extends CellBase = CellBase> = Array<
+  CellChange<Cell> & { point?: Point }
+>;
 
 export type CreateFormulaParser = (data: Matrix<CellBase>) => FormulaParser;