[FIX] CorePlugins: Prevent dispatch during adaptRange

src/helpers/pivot/pivot_registry.ts

@@ -1,6 +1,6 @@
 import { ModelConfig } from "../../model";
 import { Registry } from "../../registries/registry";
-import { CoreGetters, Getters } from "../../types";
+import { ApplyRangeChange, CoreGetters, Getters, Range } from "../../types";
 import { PivotCoreDefinition, PivotField, PivotFields } from "../../types/pivot";
 import { Pivot } from "../../types/pivot_runtime";
 import { PivotRuntimeDefinition } from "./pivot_runtime_definition";
@@ -32,6 +32,11 @@ export interface PivotRegistryItem {
   datetimeGranularities: string[];
   isMeasureCandidate: (field: PivotField) => boolean;
   isGroupable: (field: PivotField) => boolean;
+  adaptRanges?: (
+    getters: CoreGetters,
+    definition: PivotCoreDefinition,
+    applyChange: ApplyRangeChange
+  ) => PivotCoreDefinition;
 }
 
 export const pivotRegistry = new Registry<PivotRegistryItem>();
@@ -54,4 +59,40 @@ pivotRegistry.add("SPREADSHEET", {
   datetimeGranularities: [...dateGranularities, "hour_number", "minute_number", "second_number"],
   isMeasureCandidate: (field: PivotField) => !["datetime", "boolean"].includes(field.type),
   isGroupable: () => true,
+  adaptRanges: (getters, definition, applyChange) => {
+    if (definition.type !== "SPREADSHEET" || !definition.dataSet) {
+      return definition;
+    }
+    const { sheetId, zone } = definition.dataSet;
+    const range = getters.getRangeFromZone(sheetId, zone);
+    const adaptedRange = adaptPivotRange(range, applyChange);
+
+    if (adaptedRange === range) {
+      return definition;
+    }
+
+    const dataSet = adaptedRange && {
+      sheetId: adaptedRange.sheetId,
+      zone: adaptedRange.zone,
+    };
+    return { ...definition, dataSet };
+  },
 });
+
+function adaptPivotRange(
+  range: Range | undefined,
+  applyChange: ApplyRangeChange
+): Range | undefined {
+  if (!range) {
+    return undefined;
+  }
+  const change = applyChange(range);
+  switch (change.changeType) {
+    case "NONE":
+      return range;
+    case "REMOVE":
+      return undefined;
+    default:
+      return change.range;
+  }
+}

src/plugins/core/pivot.ts

@@ -1,6 +1,7 @@
 import { compile } from "../../formulas";
 import { deepCopy, deepEquals } from "../../helpers";
 import { createPivotFormula, getMaxObjectId } from "../../helpers/pivot/pivot_helpers";
+import { pivotRegistry } from "../../helpers/pivot/pivot_registry";
 import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot";
 import {
   ApplyRangeChange,
@@ -132,6 +133,18 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
   }
 
   adaptRanges(applyChange: ApplyRangeChange) {
+    for (const pivotId in this.pivots) {
+      const definition = deepCopy(this.pivots[pivotId]?.definition);
+      if (!definition) {
+        continue;
+      }
+      const newDefinition = pivotRegistry
+        .get(definition.type)
+        ?.adaptRanges?.(this.getters, definition, applyChange);
+      if (newDefinition && !deepEquals(definition, newDefinition)) {
+        this.history.update("pivots", pivotId, "definition", newDefinition);
+      }
+    }
     for (const sheetId in this.compiledMeasureFormulas) {
       for (const formulaString in this.compiledMeasureFormulas[sheetId]) {
         const compiledFormula = this.compiledMeasureFormulas[sheetId][formulaString];
@@ -314,19 +327,19 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
       if (!pivot) {
         continue;
       }
-      const def = deepCopy(pivot.definition);
 
-      for (const measure of def.measures) {
+      for (const measure of pivot.definition.measures) {
         if (measure.computedBy?.formula === formulaString) {
-          const measureIndex = def.measures.indexOf(measure);
-          if (measureIndex !== -1) {
-            def.measures[measureIndex].computedBy = {
-              formula: newFormulaString,
-              sheetId,
-            };
-          }
-
-          this.dispatch("UPDATE_PIVOT", { pivotId, pivot: def });
+          const measureIndex = pivot.definition.measures.indexOf(measure);
+          this.history.update(
+            "pivots",
+            pivotId,
+            "definition",
+            "measures",
+            measureIndex,
+            "computedBy",
+            { formula: newFormulaString, sheetId }
+          );
         }
       }
     }

src/plugins/core/range.ts

@@ -42,6 +42,7 @@ interface RangeStringOptions {
 export class RangeAdapter implements CommandHandler<CoreCommand> {
   private getters: CoreGetters;
   private providers: Array<RangeProvider["adaptRanges"]> = [];
+  private isAdaptingRanges: boolean = false;
   constructor(getters: CoreGetters) {
     this.getters = getters;
   }
@@ -73,6 +74,9 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
   beforeHandle(command: Command) {}
 
   handle(cmd: Command) {
+    if (this.isAdaptingRanges) {
+      throw new Error("Plugins cannot dispatch commands during adaptRanges phase");
+    }
     switch (cmd.type) {
       case "REMOVE_COLUMNS_ROWS": {
         let start: "left" | "top" = cmd.dimension === "COL" ? "left" : "top";
@@ -243,10 +247,12 @@ export class RangeAdapter implements CommandHandler<CoreCommand> {
   }
 
   private executeOnAllRanges(adaptRange: ApplyRangeChange, sheetId?: UID) {
+    this.isAdaptingRanges = true;
     const func = this.verifyRangeRemoved(adaptRange);
     for (const provider of this.providers) {
       provider(func, sheetId);
     }
+    this.isAdaptingRanges = false;
   }
 
   /**

src/plugins/core/spreadsheet_pivot.ts

@@ -1,31 +1,7 @@
 import { isZoneValid } from "../../helpers";
-import {
-  ApplyRangeChange,
-  CommandResult,
-  CoreCommand,
-  PivotCoreDefinition,
-  Range,
-} from "../../types";
+import { CommandResult, CoreCommand, PivotCoreDefinition } from "../../types";
 import { CorePlugin } from "../core_plugin";
 
-function adaptPivotRange(
-  range: Range | undefined,
-  applyChange: ApplyRangeChange
-): Range | undefined {
-  if (!range) {
-    return undefined;
-  }
-  const change = applyChange(range);
-  switch (change.changeType) {
-    case "NONE":
-      return range;
-    case "REMOVE":
-      return undefined;
-    default:
-      return change.range;
-  }
-}
-
 export class SpreadsheetPivotCorePlugin extends CorePlugin {
   allowDispatch(cmd: CoreCommand) {
     switch (cmd.type) {
@@ -37,30 +13,6 @@ export class SpreadsheetPivotCorePlugin extends CorePlugin {
     return CommandResult.Success;
   }
 
-  adaptRanges(applyChange: ApplyRangeChange) {
-    for (const pivotId of this.getters.getPivotIds()) {
-      const definition = this.getters.getPivotCoreDefinition(pivotId);
-      if (definition.type !== "SPREADSHEET") {
-        continue;
-      }
-      if (definition.dataSet) {
-        const { sheetId, zone } = definition.dataSet;
-        const range = this.getters.getRangeFromZone(sheetId, zone);
-        const adaptedRange = adaptPivotRange(range, applyChange);
-
-        if (adaptedRange === range) {
-          return;
-        }
-
-        const dataSet = adaptedRange && {
-          sheetId: adaptedRange.sheetId,
-          zone: adaptedRange.zone,
-        };
-        this.dispatch("UPDATE_PIVOT", { pivotId, pivot: { ...definition, dataSet } });
-      }
-    }
-  }
-
   private checkDataSetValidity(definition: PivotCoreDefinition) {
     if (definition.type === "SPREADSHEET" && definition.dataSet) {
       const { zone, sheetId } = definition.dataSet;

tests/range_plugin.test.ts

@@ -745,3 +745,17 @@ test.each([
   const adaptedRange = model.getters.createAdaptedRanges([range], 1, 1, sheetId);
   expect(model.getters.getRangeString(adaptedRange[0], sheetId)).toBe(expected);
 });
+
+test("Plugins cannot dispatch a command during adaptRanges", () => {
+  class PluginDispatchInAdaptRanges extends CorePlugin {
+    adaptRanges() {
+      this.dispatch("DELETE_SHEET", { sheetId: "s1" });
+    }
+  }
+  addTestPlugin(corePluginRegistry, PluginDispatchInAdaptRanges);
+
+  const model = new Model({});
+  expect(() => {
+    addColumns(model, "before", "A", 1);
+  }).toThrowError("Plugins cannot dispatch commands during adaptRanges phase");
+});