From fd3f63ec1809dd06f8ac3f8ef526f75288af4153 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= Date: Wed, 9 Oct 2024 14:41:06 +0200 Subject: [PATCH] [FIX] CoreViewPlugin: Remove access to dispatch The CoreView plugins have a local state that is directly derived from the core data. As such, they should not have any impact on plugins other than themselves, especially not on core plugins. This revision removes the dispatch fro the core view plugins altogether as they don't and should not dispatch anything. Task: 4241403 --- src/model.ts | 33 +++++++++++++++++-- src/plugins/base_plugin.ts | 5 +-- src/plugins/core_plugin.ts | 4 ++- src/plugins/core_view_plugin.ts | 29 ++++++++++++++++ src/plugins/index.ts | 3 +- .../cell_evaluation/evaluation_plugin.ts | 6 ++-- src/plugins/ui_core_views/custom_colors.ts | 4 +-- src/plugins/ui_core_views/evaluation_chart.ts | 4 +-- .../evaluation_conditional_format.ts | 4 +-- .../evaluation_data_validation.ts | 4 +-- src/plugins/ui_core_views/header_sizes_ui.ts | 4 +-- src/plugins/ui_plugin.ts | 4 ++- 12 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 src/plugins/core_view_plugin.ts diff --git a/src/model.ts b/src/model.ts index b182a08fb0..45201f021e 100644 --- a/src/model.ts +++ b/src/model.ts @@ -14,6 +14,11 @@ import { import { BasePlugin } from "./plugins/base_plugin"; import { RangeAdapter } from "./plugins/core/range"; import { CorePlugin, CorePluginConfig, CorePluginConstructor } from "./plugins/core_plugin"; +import { + CoreViewPlugin, + CoreViewPluginConfig, + CoreViewPluginConstructor, +} from "./plugins/core_view_plugin"; import { corePluginRegistry, coreViewsPluginRegistry, @@ -128,7 +133,7 @@ export class Model extends EventBus implements CommandDispatcher { private statefulUIPlugins: UIPlugin[] = []; - private coreViewsPlugins: UIPlugin[] = []; + private coreViewsPlugins: CoreViewPlugin[] = []; private range: RangeAdapter; @@ -159,6 +164,7 @@ export class Model extends EventBus implements CommandDispatcher { */ readonly config: ModelConfig; private corePluginConfig: CorePluginConfig; + private coreViewPluginConfig: CoreViewPluginConfig; private uiPluginConfig: UIPluginConfig; private state: StateObserver; @@ -231,6 +237,7 @@ export class Model extends EventBus implements CommandDispatcher { this.handlers.push(this.range); this.corePluginConfig = this.setupCorePluginConfig(); + this.coreViewPluginConfig = this.setupCoreViewPluginConfig(); this.uiPluginConfig = this.setupUiPluginConfig(); // registering plugins @@ -242,7 +249,7 @@ export class Model extends EventBus implements CommandDispatcher { this.session.loadInitialMessages(stateUpdateMessages); for (let Plugin of coreViewsPluginRegistry.getAll()) { - const plugin = this.setupUiPlugin(Plugin); + const plugin = this.setupCoreViewPlugin(Plugin); this.coreViewsPlugins.push(plugin); this.handlers.push(plugin); this.uiHandlers.push(plugin); @@ -309,6 +316,20 @@ export class Model extends EventBus implements CommandDispatcher { return plugin; } + private setupCoreViewPlugin(Plugin: CoreViewPluginConstructor) { + const plugin = new Plugin(this.coreViewPluginConfig); + for (let name of Plugin.getters) { + if (!(name in plugin)) { + throw new Error(`Invalid getter name: ${name} for plugin ${plugin.constructor}`); + } + if (name in this.getters) { + throw new Error(`Getter "${name}" is already defined.`); + } + this.getters[name] = plugin[name].bind(plugin); + } + return plugin; + } + /** * Initialize and properly configure a plugin. * @@ -421,6 +442,14 @@ export class Model extends EventBus implements CommandDispatcher { }; } + private setupCoreViewPluginConfig(): CoreViewPluginConfig { + return { + getters: this.getters, + stateObserver: this.state, + custom: this.config.custom, + }; + } + private setupUiPluginConfig(): UIPluginConfig { return { getters: this.getters, diff --git a/src/plugins/base_plugin.ts b/src/plugins/base_plugin.ts index e35ab7d367..870968d394 100644 --- a/src/plugins/base_plugin.ts +++ b/src/plugins/base_plugin.ts @@ -1,6 +1,5 @@ import { StateObserver } from "../state_observer"; import { - CommandDispatcher, CommandHandler, CommandResult, ExcelWorkbookData, @@ -25,14 +24,12 @@ export class BasePlugin implements CommandHandler, Vali static getters: readonly string[] = []; protected history: WorkbookHistory; - protected dispatch: CommandDispatcher["dispatch"]; - constructor(stateObserver: StateObserver, dispatch: CommandDispatcher["dispatch"]) { + constructor(stateObserver: StateObserver) { this.history = Object.assign(Object.create(stateObserver), { update: stateObserver.addChange.bind(stateObserver, this), selectCell: () => {}, }); - this.dispatch = dispatch; } /** diff --git a/src/plugins/core_plugin.ts b/src/plugins/core_plugin.ts index abc8f76120..a7b8574c1d 100644 --- a/src/plugins/core_plugin.ts +++ b/src/plugins/core_plugin.ts @@ -40,12 +40,14 @@ export class CorePlugin { protected getters: CoreGetters; protected uuidGenerator: UuidGenerator; + protected dispatch: CoreCommandDispatcher["dispatch"]; constructor({ getters, stateObserver, range, dispatch, uuidGenerator }: CorePluginConfig) { - super(stateObserver, dispatch); + super(stateObserver); range.addRangeProvider(this.adaptRanges.bind(this)); this.getters = getters; this.uuidGenerator = uuidGenerator; + this.dispatch = dispatch; } // --------------------------------------------------------------------------- diff --git a/src/plugins/core_view_plugin.ts b/src/plugins/core_view_plugin.ts new file mode 100644 index 0000000000..9def5061c6 --- /dev/null +++ b/src/plugins/core_view_plugin.ts @@ -0,0 +1,29 @@ +import { ModelConfig } from "../model"; +import { StateObserver } from "../state_observer"; +import { Command, Getters, LAYERS } from "../types/index"; +import { BasePlugin } from "./base_plugin"; + +export interface CoreViewPluginConfig { + readonly getters: Getters; + readonly stateObserver: StateObserver; + readonly custom: ModelConfig["custom"]; +} + +export interface CoreViewPluginConstructor { + new (config: CoreViewPluginConfig): CoreViewPlugin; + getters: readonly string[]; +} + +/** + * Core view plugins handle any data derived from core date (i.e. evaluation). + * They cannot impact the model data (i.e. cannot dispatch commands). + */ +export class CoreViewPlugin extends BasePlugin { + static layers: LAYERS[] = []; + + protected getters: Getters; + constructor({ getters, stateObserver }: CoreViewPluginConfig) { + super(stateObserver); + this.getters = getters; + } +} diff --git a/src/plugins/index.ts b/src/plugins/index.ts index 7da8d4ac67..c53feedb69 100644 --- a/src/plugins/index.ts +++ b/src/plugins/index.ts @@ -16,6 +16,7 @@ import { import { HeaderGroupingPlugin } from "./core/header_grouping"; import { SettingsPlugin } from "./core/settings"; import { CorePluginConstructor } from "./core_plugin"; +import { CoreViewPluginConstructor } from "./core_view_plugin"; import { CustomColorsPlugin, EvaluationChartPlugin, @@ -97,7 +98,7 @@ export const statefulUIPluginRegistry = new Registry() .add("edition", EditionPlugin); // Plugins which have a derived state from core data -export const coreViewsPluginRegistry = new Registry() +export const coreViewsPluginRegistry = new Registry() .add("evaluation", EvaluationPlugin) .add("evaluation_chart", EvaluationChartPlugin) .add("evaluation_cf", EvaluationConditionalFormatPlugin) diff --git a/src/plugins/ui_core_views/cell_evaluation/evaluation_plugin.ts b/src/plugins/ui_core_views/cell_evaluation/evaluation_plugin.ts index c2623f0dac..26381bfffd 100644 --- a/src/plugins/ui_core_views/cell_evaluation/evaluation_plugin.ts +++ b/src/plugins/ui_core_views/cell_evaluation/evaluation_plugin.ts @@ -21,7 +21,7 @@ import { invalidateDependenciesCommands, isMatrix, } from "../../../types/index"; -import { UIPlugin, UIPluginConfig } from "../../ui_plugin"; +import { CoreViewPlugin, CoreViewPluginConfig } from "../../core_view_plugin"; import { CoreViewCommand } from "./../../../types/commands"; import { Evaluator } from "./evaluator"; @@ -140,7 +140,7 @@ import { Evaluator } from "./evaluator"; // of other cells depending on it, at the next iteration. //#endregion -export class EvaluationPlugin extends UIPlugin { +export class EvaluationPlugin extends CoreViewPlugin { static getters = [ "evaluateFormula", "evaluateFormulaResult", @@ -161,7 +161,7 @@ export class EvaluationPlugin extends UIPlugin { private evaluator: Evaluator; private positionsToUpdate: CellPosition[] = []; - constructor(config: UIPluginConfig) { + constructor(config: CoreViewPluginConfig) { super(config); this.evaluator = new Evaluator(config.custom, this.getters); } diff --git a/src/plugins/ui_core_views/custom_colors.ts b/src/plugins/ui_core_views/custom_colors.ts index a5f8a411de..e08784c9e6 100644 --- a/src/plugins/ui_core_views/custom_colors.ts +++ b/src/plugins/ui_core_views/custom_colors.ts @@ -11,7 +11,7 @@ import { } from "../../helpers"; import { GaugeChart, ScorecardChart } from "../../helpers/figures/charts"; import { Color, CoreViewCommand, Immutable, RGBA, UID } from "../../types"; -import { UIPlugin } from "../ui_plugin"; +import { CoreViewPlugin } from "../core_view_plugin"; /** * https://tomekdev.com/posts/sorting-colors-in-js @@ -69,7 +69,7 @@ interface CustomColorState { * This plugins aims to compute and keep to custom colors used in the * current spreadsheet */ -export class CustomColorsPlugin extends UIPlugin { +export class CustomColorsPlugin extends CoreViewPlugin { private readonly customColors: Immutable> = {}; private readonly shouldUpdateColors = false; static getters = ["getCustomColors"] as const; diff --git a/src/plugins/ui_core_views/evaluation_chart.ts b/src/plugins/ui_core_views/evaluation_chart.ts index 0a84867dec..6d0f3a9e2b 100644 --- a/src/plugins/ui_core_views/evaluation_chart.ts +++ b/src/plugins/ui_core_views/evaluation_chart.ts @@ -7,7 +7,7 @@ import { invalidateCFEvaluationCommands, invalidateEvaluationCommands, } from "../../types/commands"; -import { UIPlugin } from "../ui_plugin"; +import { CoreViewPlugin } from "../core_view_plugin"; interface EvaluationChartStyle { background: Color; @@ -18,7 +18,7 @@ interface EvaluationChartState { charts: Record; } -export class EvaluationChartPlugin extends UIPlugin { +export class EvaluationChartPlugin extends CoreViewPlugin { static getters = ["getChartRuntime", "getStyleOfSingleCellChart"] as const; charts: Record = {}; diff --git a/src/plugins/ui_core_views/evaluation_conditional_format.ts b/src/plugins/ui_core_views/evaluation_conditional_format.ts index 4d7e4e74d1..14f357691b 100644 --- a/src/plugins/ui_core_views/evaluation_conditional_format.ts +++ b/src/plugins/ui_core_views/evaluation_conditional_format.ts @@ -24,13 +24,13 @@ import { invalidateCFEvaluationCommands, isMatrix, } from "../../types/index"; -import { UIPlugin } from "../ui_plugin"; +import { CoreViewPlugin } from "../core_view_plugin"; import { CoreViewCommand } from "./../../types/commands"; type ComputedStyles = { [col: HeaderIndex]: (Style | undefined)[] }; type ComputedIcons = { [col: HeaderIndex]: (string | undefined)[] }; -export class EvaluationConditionalFormatPlugin extends UIPlugin { +export class EvaluationConditionalFormatPlugin extends CoreViewPlugin { static getters = ["getConditionalIcon", "getCellComputedStyle"] as const; private isStale: boolean = true; // stores the computed styles in the format of computedStyles.sheetName[col][row] = Style diff --git a/src/plugins/ui_core_views/evaluation_data_validation.ts b/src/plugins/ui_core_views/evaluation_data_validation.ts index ad392223ba..93507a3fda 100644 --- a/src/plugins/ui_core_views/evaluation_data_validation.ts +++ b/src/plugins/ui_core_views/evaluation_data_validation.ts @@ -16,7 +16,7 @@ import { } from "../../types"; import { CoreViewCommand, invalidateEvaluationCommands } from "../../types/commands"; import { CellErrorType, EvaluationError } from "../../types/errors"; -import { UIPlugin } from "../ui_plugin"; +import { CoreViewPlugin } from "../core_view_plugin"; import { _t } from "./../../translation"; interface InvalidValidationResult { @@ -35,7 +35,7 @@ type ValidationResult = ValidValidationResult | InvalidValidationResult; type SheetValidationResult = { [col: HeaderIndex]: Array> }; -export class EvaluationDataValidationPlugin extends UIPlugin { +export class EvaluationDataValidationPlugin extends CoreViewPlugin { static getters = [ "getDataValidationInvalidCriterionValueMessage", "getInvalidDataValidationMessage", diff --git a/src/plugins/ui_core_views/header_sizes_ui.ts b/src/plugins/ui_core_views/header_sizes_ui.ts index be9c3e6758..5fef60a247 100644 --- a/src/plugins/ui_core_views/header_sizes_ui.ts +++ b/src/plugins/ui_core_views/header_sizes_ui.ts @@ -10,7 +10,7 @@ import { } from "../../helpers"; import { Command } from "../../types"; import { CellPosition, Dimension, HeaderIndex, Immutable, Pixel, UID } from "../../types/misc"; -import { UIPlugin } from "../ui_plugin"; +import { CoreViewPlugin } from "../core_view_plugin"; interface HeaderSizeState { tallestCellInRow: Immutable>>; @@ -21,7 +21,7 @@ interface CellWithSize { size: Pixel; } -export class HeaderSizeUIPlugin extends UIPlugin implements HeaderSizeState { +export class HeaderSizeUIPlugin extends CoreViewPlugin implements HeaderSizeState { static getters = ["getRowSize", "getHeaderSize"] as const; readonly tallestCellInRow: Immutable>> = {}; diff --git a/src/plugins/ui_plugin.ts b/src/plugins/ui_plugin.ts index b9cc22f2b9..b506f01fa5 100644 --- a/src/plugins/ui_plugin.ts +++ b/src/plugins/ui_plugin.ts @@ -43,11 +43,13 @@ export class UIPlugin extends BasePlugin { protected getters: Getters; protected ui: UIActions; protected selection: SelectionStreamProcessor; + protected dispatch: CommandDispatcher["dispatch"]; constructor({ getters, stateObserver, dispatch, uiActions, selection }: UIPluginConfig) { - super(stateObserver, dispatch); + super(stateObserver); this.getters = getters; this.ui = uiActions; this.selection = selection; + this.dispatch = dispatch; } // ---------------------------------------------------------------------------