Skip to content

Commit

Permalink
[FIX] CoreViewPlugin: Remove access to dispatch
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rrahir committed Nov 14, 2024
1 parent 6cc0b14 commit 05278ef
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 22 deletions.
38 changes: 36 additions & 2 deletions src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -128,7 +133,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {

private statefulUIPlugins: UIPlugin[] = [];

private coreViewsPlugins: UIPlugin[] = [];
private coreViewsPlugins: CoreViewPlugin[] = [];

private range: RangeAdapter;

Expand Down Expand Up @@ -159,6 +164,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
*/
readonly config: ModelConfig;
private corePluginConfig: CorePluginConfig;
private coreViewPluginConfig: CoreViewPluginConfig;
private uiPluginConfig: UIPluginConfig;

private state: StateObserver;
Expand Down Expand Up @@ -231,6 +237,7 @@ export class Model extends EventBus<any> implements CommandDispatcher {
this.handlers.push(this.range);

this.corePluginConfig = this.setupCorePluginConfig();
this.coreViewPluginConfig = this.setupCoreViewPluginConfig();
this.uiPluginConfig = this.setupUiPluginConfig();

// registering plugins
Expand All @@ -242,7 +249,7 @@ export class Model extends EventBus<any> 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);
Expand Down Expand Up @@ -309,6 +316,20 @@ export class Model extends EventBus<any> 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.
*
Expand Down Expand Up @@ -421,6 +442,19 @@ export class Model extends EventBus<any> implements CommandDispatcher {
};
}

private setupCoreViewPluginConfig(): CoreViewPluginConfig {
return {
getters: this.getters,
stateObserver: this.state,
selection: this.selection,
moveClient: this.session.move.bind(this.session),
custom: this.config.custom,
uiActions: this.config,
session: this.session,
defaultCurrencyFormat: this.config.defaultCurrencyFormat,
};
}

private setupUiPluginConfig(): UIPluginConfig {
return {
getters: this.getters,
Expand Down
5 changes: 1 addition & 4 deletions src/plugins/base_plugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { StateObserver } from "../state_observer";
import {
CommandDispatcher,
CommandHandler,
CommandResult,
ExcelWorkbookData,
Expand All @@ -25,14 +24,12 @@ export class BasePlugin<State = any, C = any> implements CommandHandler<C>, Vali
static getters: readonly string[] = [];

protected history: WorkbookHistory<State>;
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;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/core_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ export class CorePlugin<State = any>
{
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;
}

// ---------------------------------------------------------------------------
Expand Down
24 changes: 24 additions & 0 deletions src/plugins/core_view_plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Command, Getters, LAYERS } from "../types/index";
import { BasePlugin } from "./base_plugin";
import { UIPluginConfig } from "./ui_plugin";

export type CoreViewPluginConfig = Omit<UIPluginConfig, "dispatch">;

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<State = any> extends BasePlugin<State, Command> {
static layers: LAYERS[] = [];

protected getters: Getters;
constructor({ getters, stateObserver }: CoreViewPluginConfig) {
super(stateObserver);
this.getters = getters;
}
}
3 changes: 2 additions & 1 deletion src/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -97,7 +98,7 @@ export const statefulUIPluginRegistry = new Registry<UIPluginConstructor>()
.add("edition", EditionPlugin);

// Plugins which have a derived state from core data
export const coreViewsPluginRegistry = new Registry<UIPluginConstructor>()
export const coreViewsPluginRegistry = new Registry<CoreViewPluginConstructor>()
.add("evaluation", EvaluationPlugin)
.add("evaluation_chart", EvaluationChartPlugin)
.add("evaluation_cf", EvaluationConditionalFormatPlugin)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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",
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/ui_core_views/custom_colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<CustomColorState> {
export class CustomColorsPlugin extends CoreViewPlugin<CustomColorState> {
private readonly customColors: Immutable<Record<Color, true>> = {};
private readonly shouldUpdateColors = false;
static getters = ["getCustomColors"] as const;
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/ui_core_views/evaluation_chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,7 +18,7 @@ interface EvaluationChartState {
charts: Record<UID, ChartRuntime | undefined>;
}

export class EvaluationChartPlugin extends UIPlugin<EvaluationChartState> {
export class EvaluationChartPlugin extends CoreViewPlugin<EvaluationChartState> {
static getters = ["getChartRuntime", "getStyleOfSingleCellChart"] as const;

charts: Record<UID, ChartRuntime | undefined> = {};
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/ui_core_views/evaluation_conditional_format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/ui_core_views/evaluation_data_validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -35,7 +35,7 @@ type ValidationResult = ValidValidationResult | InvalidValidationResult;

type SheetValidationResult = { [col: HeaderIndex]: Array<Lazy<ValidationResult>> };

export class EvaluationDataValidationPlugin extends UIPlugin {
export class EvaluationDataValidationPlugin extends CoreViewPlugin {
static getters = [
"getDataValidationInvalidCriterionValueMessage",
"getInvalidDataValidationMessage",
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/ui_core_views/header_sizes_ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<UID, Array<CellWithSize | undefined>>>;
Expand All @@ -21,7 +21,7 @@ interface CellWithSize {
size: Pixel;
}

export class HeaderSizeUIPlugin extends UIPlugin<HeaderSizeState> implements HeaderSizeState {
export class HeaderSizeUIPlugin extends CoreViewPlugin<HeaderSizeState> implements HeaderSizeState {
static getters = ["getRowSize", "getHeaderSize"] as const;

readonly tallestCellInRow: Immutable<Record<UID, Array<CellWithSize | undefined>>> = {};
Expand Down
4 changes: 3 additions & 1 deletion src/plugins/ui_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ export class UIPlugin<State = any> extends BasePlugin<State, Command> {
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;
}

// ---------------------------------------------------------------------------
Expand Down

0 comments on commit 05278ef

Please sign in to comment.