Skip to content

Commit

Permalink
[IMP] 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 18, 2024
1 parent 5222d9a commit b93be84
Show file tree
Hide file tree
Showing 15 changed files with 105 additions and 35 deletions.
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export { findCellInNewZone } from "./helpers/zones";
export { load } from "./migrations/data";
export { Model } from "./model";
export { CorePlugin } from "./plugins/core_plugin";
export { CoreViewPlugin } from "./plugins/core_view_plugin";
export { UIPlugin } from "./plugins/ui_plugin";
export { Registry } from "./registries/registry";
export { setTranslationMethod } from "./translation";
Expand Down
34 changes: 33 additions & 1 deletion src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { BasePlugin } from "./plugins/base_plugin";
import { RangeAdapter } from "./plugins/core/range";
import { CorePlugin, CorePluginConfig, CorePluginConstructor } from "./plugins/core_plugin";
import { CoreViewPluginConfig, CoreViewPluginConstructor } from "./plugins/core_view_plugin";
import {
corePluginRegistry,
coreViewsPluginRegistry,
Expand Down Expand Up @@ -124,6 +125,7 @@ const enum Status {

export class Model extends EventBus<any> implements CommandDispatcher {
private corePlugins: CorePlugin[] = [];

private statefulUIPlugins: UIPlugin[] = [];

private range: RangeAdapter;
Expand Down Expand Up @@ -155,6 +157,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 @@ -232,6 +235,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 @@ -243,7 +247,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.handlers.push(plugin);
this.uiHandlers.push(plugin);
this.coreHandlers.push(plugin);
Expand Down Expand Up @@ -315,6 +319,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 @@ -427,6 +445,20 @@ 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,
defaultCurrency: this.config.defaultCurrency,
customColors: this.config.customColors || [],
};
}

private setupUiPluginConfig(): UIPluginConfig {
return {
getters: this.getters,
Expand Down
11 changes: 1 addition & 10 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,20 +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"];
protected canDispatch: CommandDispatcher["dispatch"];

constructor(
stateObserver: StateObserver,
dispatch: CommandDispatcher["dispatch"],
canDispatch: CommandDispatcher["dispatch"]
) {
constructor(stateObserver: StateObserver) {
this.history = Object.assign(Object.create(stateObserver), {
update: stateObserver.addChange.bind(stateObserver, this),
selectCell: () => {},
});
this.dispatch = dispatch;
this.canDispatch = canDispatch;
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/core_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ export class CorePlugin<State = any>
implements RangeProvider
{
protected getters: CoreGetters;
protected dispatch: CoreCommandDispatcher["dispatch"];
protected canDispatch: CoreCommandDispatcher["dispatch"];

constructor({ getters, stateObserver, range, dispatch, canDispatch }: CorePluginConfig) {
super(stateObserver, dispatch, canDispatch);
super(stateObserver);
range.addRangeProvider(this.adaptRanges.bind(this));
this.getters = getters;
this.dispatch = dispatch;
this.canDispatch = canDispatch;
}

// ---------------------------------------------------------------------------
Expand Down
36 changes: 36 additions & 0 deletions src/plugins/core_view_plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { Session } from "../collaborative/session";
import { ModelConfig } from "../model";
import { SelectionStreamProcessor } from "../selection_stream/selection_stream_processor";
import { StateObserver } from "../state_observer";
import { ClientPosition, Color, Command, Currency, Getters } from "../types/index";
import { BasePlugin } from "./base_plugin";
import { UIActions } from "./ui_plugin";

export interface CoreViewPluginConfig {
readonly getters: Getters;
readonly stateObserver: StateObserver;
readonly selection: SelectionStreamProcessor;
readonly moveClient: (position: ClientPosition) => void;
readonly uiActions: UIActions;
readonly custom: ModelConfig["custom"];
readonly session: Session;
readonly defaultCurrency?: Partial<Currency>;
readonly customColors: Color[];
}

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> {
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 @@ -19,6 +19,7 @@ import { SettingsPlugin } from "./core/settings";
import { SpreadsheetPivotCorePlugin } from "./core/spreadsheet_pivot";
import { TableStylePlugin } from "./core/table_style";
import { CorePluginConstructor } from "./core_plugin";
import { CoreViewPluginConstructor } from "./core_view_plugin";
import {
CustomColorsPlugin,
EvaluationChartPlugin,
Expand Down Expand Up @@ -105,7 +106,7 @@ export const statefulUIPluginRegistry = new Registry<UIPluginConstructor>()
.add("clipboard", ClipboardPlugin);

// 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 @@ -22,7 +22,7 @@ import {
isMatrix,
} from "../../../types/index";
import { FormulaCellWithDependencies } from "../../core";
import { UIPlugin, UIPluginConfig } from "../../ui_plugin";
import { CoreViewPlugin, CoreViewPluginConfig } from "../../core_view_plugin";
import { CoreViewCommand, invalidateEvaluationCommands } from "./../../../types/commands";
import { Evaluator } from "./evaluator";

Expand Down Expand Up @@ -141,7 +141,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 @@ -164,7 +164,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
6 changes: 3 additions & 3 deletions src/plugins/ui_core_views/custom_colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
toHex,
} from "../../helpers";
import { Color, Command, Immutable, RGBA, TableElementStyle, UID } from "../../types";
import { UIPlugin, UIPluginConfig } from "../ui_plugin";
import { CoreViewPlugin, CoreViewPluginConfig } from "../core_view_plugin";

const chartColorRegex = /"(#[0-9a-fA-F]{6})"/g;

Expand Down Expand Up @@ -70,12 +70,12 @@ 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 = true;
static getters = ["getCustomColors"] as const;

constructor(config: UIPluginConfig) {
constructor(config: CoreViewPluginConfig) {
super(config);
this.tryToAddColors(config.customColors ?? []);
}
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/ui_core_views/dynamic_tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ import {
Zone,
invalidateEvaluationCommands,
} from "../../types/index";
import { CoreViewPlugin } from "../core_view_plugin";

import { UIPlugin } from "../ui_plugin";

export class DynamicTablesPlugin extends UIPlugin {
export class DynamicTablesPlugin extends CoreViewPlugin {
static getters = [
"canCreateDynamicTableOnZones",
"doesZonesContainFilter",
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 @@ -8,7 +8,7 @@ import {
invalidateChartEvaluationCommands,
invalidateEvaluationCommands,
} from "../../types/commands";
import { UIPlugin } from "../ui_plugin";
import { CoreViewPlugin } from "../core_view_plugin";

interface EvaluationChartStyle {
background: Color;
Expand All @@ -19,7 +19,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 @@ -25,14 +25,14 @@ import {
invalidateCFEvaluationCommands,
isMatrix,
} from "../../types/index";
import { UIPlugin } from "../ui_plugin";
import { CoreViewPlugin } from "../core_view_plugin";
import { CoreViewCommand, invalidateEvaluationCommands } from "./../../types/commands";

type ComputedStyles = { [col: HeaderIndex]: (Style | undefined)[] };
type ComputedIcons = { [col: HeaderIndex]: (string | undefined)[] };
type ComputedDataBars = { [col: HeaderIndex]: (DataBarFill | undefined)[] };

export class EvaluationConditionalFormatPlugin extends UIPlugin {
export class EvaluationConditionalFormatPlugin extends CoreViewPlugin {
static getters = [
"getConditionalIcon",
"getCellConditionalFormatStyle",
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 @@ -15,7 +15,7 @@ import {
isMatrix,
} from "../../types";
import { CoreViewCommand, invalidateEvaluationCommands } from "../../types/commands";
import { UIPlugin } from "../ui_plugin";
import { CoreViewPlugin } from "../core_view_plugin";
import { _t } from "./../../translation";

interface InvalidValidationResult {
Expand All @@ -34,7 +34,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
7 changes: 4 additions & 3 deletions src/plugins/ui_core_views/pivot_ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ import {
invalidateEvaluationCommands,
} from "../../types";
import { Pivot } from "../../types/pivot_runtime";
import { UIPlugin, UIPluginConfig } from "../ui_plugin";
import { CoreViewPlugin, CoreViewPluginConfig } from "../core_view_plugin";
import { UIPluginConfig } from "../ui_plugin";

export const UNDO_REDO_PIVOT_COMMANDS = ["ADD_PIVOT", "UPDATE_PIVOT"];

function isPivotCommand(cmd: CoreCommand): cmd is AddPivotCommand | UpdatePivotCommand {
return UNDO_REDO_PIVOT_COMMANDS.includes(cmd.type);
}

export class PivotUIPlugin extends UIPlugin {
export class PivotUIPlugin extends CoreViewPlugin {
static getters = [
"getPivot",
"getFirstPivotFunction",
Expand All @@ -45,7 +46,7 @@ export class PivotUIPlugin extends UIPlugin {
private unusedPivots?: UID[];
private custom: UIPluginConfig["custom"];

constructor(config: UIPluginConfig) {
constructor(config: CoreViewPluginConfig) {
super(config);
this.custom = config.custom;
}
Expand Down
9 changes: 7 additions & 2 deletions src/plugins/ui_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from "../types/index";
import { BasePlugin } from "./base_plugin";

type UIActions = Pick<ModelConfig, "notifyUI" | "raiseBlockingErrorUI">;
export type UIActions = Pick<ModelConfig, "notifyUI" | "raiseBlockingErrorUI">;

export interface UIPluginConfig {
readonly getters: Getters;
Expand Down Expand Up @@ -46,6 +46,9 @@ export class UIPlugin<State = any> extends BasePlugin<State, Command> {
protected getters: Getters;
protected ui: UIActions;
protected selection: SelectionStreamProcessor;
protected dispatch: CommandDispatcher["dispatch"];
protected canDispatch: CommandDispatcher["dispatch"];

constructor({
getters,
stateObserver,
Expand All @@ -54,10 +57,12 @@ export class UIPlugin<State = any> extends BasePlugin<State, Command> {
uiActions,
selection,
}: UIPluginConfig) {
super(stateObserver, dispatch, canDispatch);
super(stateObserver);
this.getters = getters;
this.ui = uiActions;
this.selection = selection;
this.dispatch = dispatch;
this.canDispatch = canDispatch;
}

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

0 comments on commit b93be84

Please sign in to comment.