Skip to content

Commit

Permalink
[REF] plugins: remove core uuid generators
Browse files Browse the repository at this point in the history
Using uuids in core plugins is either:

- wrong if the id is supposed to be shared across users since a different one
  would be generated for each users.
- useless if they aren't shared: the id doesn't need to be unique.

This commit removes the `uuidGenerator` from core plugins because it shouldn't
be used.

The use in the Table plugin was useless (the id is never shared, it's not part
of any table core command)

closes #5030

Task: 4216816
Signed-off-by: Vincent Schippefilt (vsc) <[email protected]>
  • Loading branch information
LucasLefevre committed Oct 7, 2024
1 parent 703f2a4 commit 6de1c1f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 18 deletions.
1 change: 0 additions & 1 deletion src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ export class Model extends EventBus<any> implements CommandDispatcher {
range: this.range,
dispatch: this.dispatchFromCorePlugin,
canDispatch: this.canDispatch,
uuidGenerator: this.uuidGenerator,
custom: this.config.custom,
external: this.config.external,
};
Expand Down
12 changes: 7 additions & 5 deletions src/plugins/core/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import {

import { CorePlugin } from "../core_plugin";

let nextTableId = 1;

interface TableState {
tables: Record<UID, Record<TableId, CoreTable | undefined>>;
}
Expand Down Expand Up @@ -126,7 +128,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
const mergesInTarget = this.getters.getMergesInZone(cmd.sheetId, union.zone);
this.dispatch("REMOVE_MERGE", { sheetId: cmd.sheetId, target: mergesInTarget });

const id = this.uuidGenerator.uuidv4();
const id = `${nextTableId++}`;
const config = cmd.config || DEFAULT_TABLE_CONFIG;
const newTable =
cmd.tableType === "dynamic"
Expand Down Expand Up @@ -310,7 +312,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
filters = [];
for (const i of range(zone.left, zone.right + 1)) {
const filterZone = { ...zone, left: i, right: i };
const uid = this.uuidGenerator.uuidv4();
const uid = `${nextTableId++}`;
filters.push(this.createFilterFromZone(uid, tableRange.sheetId, filterZone, config));
}
}
Expand Down Expand Up @@ -391,7 +393,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
? table.filters.find((f) => f.col === i)
: undefined;
const filterZone = { ...tableZone, left: i, right: i };
const filterId = oldFilter?.id || this.uuidGenerator.uuidv4();
const filterId = oldFilter?.id || `${nextTableId++}`;
filters.push(this.createFilterFromZone(filterId, tableRange.sheetId, filterZone, config));
}
}
Expand Down Expand Up @@ -514,7 +516,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
if (filters.length < zoneToDimension(tableZone).numberOfCols) {
for (let col = tableZone.left; col <= tableZone.right; col++) {
if (!filters.find((filter) => filter.col === col)) {
const uid = this.uuidGenerator.uuidv4();
const uid = `${nextTableId++}`;
const filterZone = { ...tableZone, left: col, right: col };
filters.push(this.createFilterFromZone(uid, sheetId, filterZone, table.config));
}
Expand All @@ -539,7 +541,7 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
import(data: WorkbookData) {
for (const sheet of data.sheets) {
for (const tableData of sheet.tables || []) {
const uuid = this.uuidGenerator.uuidv4();
const uuid = `${nextTableId++}`;
const tableConfig = tableData.config || DEFAULT_TABLE_CONFIG;
const range = this.getters.getRangeFromSheetXC(sheet.id, tableData.range);
const tableType = tableData.type || "static";
Expand Down
13 changes: 1 addition & 12 deletions src/plugins/core_plugin.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { UuidGenerator } from "../helpers";
import { ModelConfig } from "../model";
import { StateObserver } from "../state_observer";
import {
Expand All @@ -19,7 +18,6 @@ export interface CorePluginConfig {
readonly range: RangeAdapter;
readonly dispatch: CoreCommandDispatcher["dispatch"];
readonly canDispatch: CoreCommandDispatcher["dispatch"];
readonly uuidGenerator: UuidGenerator;
readonly custom: ModelConfig["custom"];
readonly external: ModelConfig["external"];
}
Expand All @@ -40,20 +38,11 @@ export class CorePlugin<State = any>
implements RangeProvider
{
protected getters: CoreGetters;
protected uuidGenerator: UuidGenerator;

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

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

0 comments on commit 6de1c1f

Please sign in to comment.