Skip to content

Commit c803393

Browse files
authored
feat(tree): Add range-based removal methods to TableSchema (microsoft#25235)
Adds range-based overloads to `removeColumns` and `removeRows` for removing contiguous ranges of rows and columns.
1 parent fd12c96 commit c803393

File tree

6 files changed

+292
-76
lines changed

6 files changed

+292
-76
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@fluidframework/tree": minor
3+
"fluid-framework": minor
4+
"__section": tree
5+
---
6+
Range-based row/column removal methods have been added to TableSchema APIs (alpha)
7+
8+
Adds range-based overloads to `removeColumns` and `removeRows` for removing contiguous ranges of rows and columns.
9+
10+
The `removeAllColumns` and `removeAllRows` methods have been removed, as they can be trivially implemented in terms of the new methods.

examples/data-objects/table-tree/src/react/tableView.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ export const TableView: React.FC<{ tableModel: TableDataObject }> = ({ tableMode
5454

5555
const handleRemoveRow = (index: number): void => {
5656
if (index >= 0 && index < rows.length) {
57-
// TODO: use index-based removal API once that has been added.
58-
table.removeRows([table.rows[index]]);
57+
table.removeRows(index, 1);
5958
}
6059
};
6160

@@ -67,8 +66,7 @@ export const TableView: React.FC<{ tableModel: TableDataObject }> = ({ tableMode
6766

6867
const handleRemoveColumn = (index: number): void => {
6968
if (index >= 0 && index < columns.length) {
70-
// TODO: use index-based removal API once that has been added.
71-
table.removeColumns([table.columns[index]]);
69+
table.removeColumns(index, 1);
7270
}
7371
};
7472

packages/dds/tree/api-report/tree.alpha.api.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,11 +1190,11 @@ export namespace TableSchema {
11901190
getRow(id: string): TreeNodeFromImplicitAllowedTypes<TRow> | undefined;
11911191
insertColumns(params: InsertColumnsParameters<TColumn>): TreeNodeFromImplicitAllowedTypes<TColumn>[];
11921192
insertRows(params: InsertRowsParameters<TRow>): TreeNodeFromImplicitAllowedTypes<TRow>[];
1193-
removeAllColumns(): TreeNodeFromImplicitAllowedTypes<TColumn>[];
1194-
removeAllRows(): TreeNodeFromImplicitAllowedTypes<TRow>[];
11951193
removeCell(key: CellKey<TColumn, TRow>): TreeNodeFromImplicitAllowedTypes<TCell> | undefined;
1194+
removeColumns(index?: number | undefined, count?: number | undefined): TreeNodeFromImplicitAllowedTypes<TColumn>[];
11961195
removeColumns(columns: readonly TreeNodeFromImplicitAllowedTypes<TColumn>[]): TreeNodeFromImplicitAllowedTypes<TColumn>[];
11971196
removeColumns(columns: readonly string[]): TreeNodeFromImplicitAllowedTypes<TColumn>[];
1197+
removeRows(index?: number | undefined, count?: number | undefined): TreeNodeFromImplicitAllowedTypes<TRow>[];
11981198
removeRows(rows: readonly TreeNodeFromImplicitAllowedTypes<TRow>[]): TreeNodeFromImplicitAllowedTypes<TRow>[];
11991199
removeRows(rows: readonly string[]): TreeNodeFromImplicitAllowedTypes<TRow>[];
12001200
readonly rows: TreeArrayNode<TRow>;

packages/dds/tree/src/tableSchema.ts

Lines changed: 118 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License.
44
*/
55

6-
import { assert, oob } from "@fluidframework/core-utils/internal";
6+
import { assert } from "@fluidframework/core-utils/internal";
77
import { UsageError } from "@fluidframework/telemetry-utils/internal";
88

99
import { Tree, TreeAlpha } from "./shared-tree/index.js";
@@ -723,27 +723,34 @@ export namespace System_TableSchema {
723723
}
724724

725725
public removeColumns(
726-
columns: readonly string[] | readonly ColumnValueType[],
726+
indexOrColumns: number | undefined | readonly string[] | readonly ColumnValueType[],
727+
count: number | undefined = undefined,
727728
): ColumnValueType[] {
728-
// If there are no columns to remove, do nothing
729-
if (columns.length === 0) {
730-
return [];
729+
if (typeof indexOrColumns === "number" || indexOrColumns === undefined) {
730+
const startIndex = indexOrColumns ?? 0;
731+
return Table._removeRange(
732+
{
733+
index: startIndex,
734+
count: count ?? this.columns.length - startIndex,
735+
},
736+
this.columns,
737+
);
731738
}
732739

733-
// If there is only one column to remove, remove it (and don't incur cost of transaction)
734-
if (columns.length === 1) {
735-
const removedColumn = this.removeColumn(columns[0] ?? oob());
736-
return [removedColumn];
740+
const columnsToRemove = indexOrColumns;
741+
742+
// If there are no columns to remove, do nothing
743+
if (columnsToRemove.length === 0) {
744+
return [];
737745
}
738746

739-
// If there are multiple columns to remove, remove them in a transaction.
740747
const removedColumns: ColumnValueType[] = [];
741748
Tree.runTransaction(this, () => {
742749
// Note, throwing an error within a transaction will abort the entire transaction.
743-
// So if we throw an error here for any row, no columns will be removed.
744-
for (const columnToRemove of columns) {
745-
const removedRow = this.removeColumn(columnToRemove);
746-
removedColumns.push(removedRow);
750+
// So if we throw an error here for any column, no columns will be removed.
751+
for (const columnToRemove of columnsToRemove) {
752+
const removedColumn = this.removeColumn(columnToRemove);
753+
removedColumns.push(removedColumn);
747754
}
748755
});
749756
return removedColumns;
@@ -774,30 +781,33 @@ export namespace System_TableSchema {
774781
return column;
775782
}
776783

777-
public removeAllColumns(): ColumnValueType[] {
778-
// TypeScript is unable to narrow the row type correctly here, hence the cast.
779-
// See: https://github.com/microsoft/TypeScript/issues/52144
780-
return this.removeColumns(this.columns as unknown as ColumnValueType[]);
781-
}
784+
public removeRows(
785+
indexOrRows: number | undefined | readonly string[] | readonly RowValueType[],
786+
count?: number | undefined,
787+
): RowValueType[] {
788+
if (typeof indexOrRows === "number" || indexOrRows === undefined) {
789+
const startIndex = indexOrRows ?? 0;
790+
return Table._removeRange(
791+
{
792+
index: startIndex,
793+
count: count ?? this.rows.length - startIndex,
794+
},
795+
this.rows,
796+
);
797+
}
798+
799+
const rowsToRemove = indexOrRows;
782800

783-
public removeRows(rows: readonly string[] | readonly RowValueType[]): RowValueType[] {
784801
// If there are no rows to remove, do nothing
785-
if (rows.length === 0) {
802+
if (rowsToRemove.length === 0) {
786803
return [];
787804
}
788805

789-
// If there is only one row to remove, remove it (and don't incur cost of transaction)
790-
if (rows.length === 1) {
791-
const removedRow = this.removeRow(rows[0] ?? oob());
792-
return [removedRow];
793-
}
794-
795-
// If there are multiple rows to remove, remove them in a transaction.
796806
const removedRows: RowValueType[] = [];
797807
Tree.runTransaction(this, () => {
798808
// Note, throwing an error within a transaction will abort the entire transaction.
799809
// So if we throw an error here for any row, no rows will be removed.
800-
for (const rowToRemove of rows) {
810+
for (const rowToRemove of rowsToRemove) {
801811
const removedRow = this.removeRow(rowToRemove);
802812
removedRows.push(removedRow);
803813
}
@@ -821,12 +831,6 @@ export namespace System_TableSchema {
821831
return rowToRemove as RowValueType;
822832
}
823833

824-
public removeAllRows(): RowValueType[] {
825-
// TypeScript is unable to narrow the row type correctly here, hence the cast.
826-
// See: https://github.com/microsoft/TypeScript/issues/52144
827-
return this.removeRows(this.rows as unknown as RowValueType[]);
828-
}
829-
830834
public removeCell(
831835
key: TableSchema.CellKey<TColumnSchema, TRowSchema>,
832836
): CellValueType | undefined {
@@ -864,6 +868,20 @@ export namespace System_TableSchema {
864868
return typeof columnOrId === "string" ? columnOrId : columnOrId.id;
865869
}
866870

871+
private _getColumnIndex(columnOrId: string | ColumnValueType): number | undefined {
872+
const column = this._getColumn(columnOrId);
873+
if (column === undefined) {
874+
return undefined;
875+
}
876+
877+
const index = this.columns.indexOf(column);
878+
return index === -1 ? undefined : index;
879+
}
880+
881+
private containsColumnWithId(columnId: string): boolean {
882+
return this._getColumnIndex(columnId) !== undefined;
883+
}
884+
867885
private _getRow(rowOrId: string | RowValueType): RowValueType | undefined {
868886
return typeof rowOrId === "string" ? this.getRow(rowOrId) : rowOrId;
869887
}
@@ -872,23 +890,50 @@ export namespace System_TableSchema {
872890
return typeof rowOrId === "string" ? rowOrId : rowOrId.id;
873891
}
874892

875-
private containsColumnWithId(columnId: string): boolean {
876-
// TypeScript is unable to narrow the types correctly here, hence the cast.
877-
// See: https://github.com/microsoft/TypeScript/issues/52144
878-
return (
879-
this.columns.find(
880-
(column) => (column as TableSchema.Column<TCellSchema>).id === columnId,
881-
) !== undefined
882-
);
893+
private _getRowIndex(rowOrId: string | RowValueType): number | undefined {
894+
const row = this._getRow(rowOrId);
895+
if (row === undefined) {
896+
return undefined;
897+
}
898+
const index = this.rows.indexOf(row);
899+
return index === -1 ? undefined : index;
883900
}
884901

885902
private containsRowWithId(rowId: string): boolean {
886-
// TypeScript is unable to narrow the types correctly here, hence the cast.
903+
return this._getRowIndex(rowId) !== undefined;
904+
}
905+
906+
private static _removeRange<TNodeSchema extends ImplicitAllowedTypes>(
907+
range: { index: number; count: number },
908+
array: TreeArrayNode<TNodeSchema>,
909+
): TreeNodeFromImplicitAllowedTypes<TNodeSchema>[] {
910+
const { index, count } = range;
911+
912+
if (index < 0 || index >= array.length) {
913+
throw new UsageError(
914+
`Start index out of bounds. Expected index to be on [0, ${array.length - 1}], but got ${index}.`,
915+
);
916+
}
917+
if (count < 0) {
918+
throw new UsageError(`Expected non-negative count. Got ${count}.`);
919+
}
920+
921+
const end = index + count; // exclusive
922+
if (end > array.length) {
923+
throw new UsageError(
924+
`End index out of bounds. Expected end to be on [${index}, ${array.length}], but got ${end}.`,
925+
);
926+
}
927+
928+
// TypeScript is unable to narrow the array element type correctly here, hence the cast.
887929
// See: https://github.com/microsoft/TypeScript/issues/52144
888-
return (
889-
this.rows.find((row) => (row as TableSchema.Row<TCellSchema>).id === rowId) !==
890-
undefined
891-
);
930+
const removedRows = array.slice(
931+
index,
932+
end,
933+
) as TreeNodeFromImplicitAllowedTypes<TNodeSchema>[];
934+
array.removeRange(index, end);
935+
936+
return removedRows;
892937
}
893938

894939
/**
@@ -1436,6 +1481,23 @@ export namespace TableSchema {
14361481
*/
14371482
setCell(params: SetCellParameters<TCell, TColumn, TRow>): void;
14381483

1484+
/**
1485+
* Removes a range of columns from the table.
1486+
*
1487+
* @remarks
1488+
* Also removes any corresponding cells from the table's rows.
1489+
*
1490+
* Note: this operation can be slow for tables with many rows.
1491+
* We are actively working on improving the performance of this operation, but for now it may have a negative
1492+
* impact on performance.
1493+
* @param index - The starting index of the range to remove. Default: `0`.
1494+
* @param count - The number of columns to remove. Default: all remaining columns starting from `index`.
1495+
* @throws Throws an error if the specified range is invalid. In this case, no columns are removed.
1496+
*/
1497+
removeColumns(
1498+
index?: number | undefined,
1499+
count?: number | undefined,
1500+
): TreeNodeFromImplicitAllowedTypes<TColumn>[];
14391501
/**
14401502
* Removes 0 or more columns from the table.
14411503
* @remarks
@@ -1469,19 +1531,15 @@ export namespace TableSchema {
14691531
removeColumns(columns: readonly string[]): TreeNodeFromImplicitAllowedTypes<TColumn>[];
14701532

14711533
/**
1472-
* Removes all columns from the table.
1473-
*
1474-
* @remarks
1475-
* Also removes any corresponding cells from the table's rows.
1476-
*
1477-
* Note: this operation can be slow for tables with many rows.
1478-
* We are actively working on improving the performance of this operation, but for now it may have a negative
1479-
* impact on performance.
1480-
*
1481-
* @returns The removed columns.
1534+
* Removes a range of rows from the table.
1535+
* @param index - The starting index of the range to remove. Default: `0`.
1536+
* @param count - The number of rows to remove. Default: all remaining rows starting from `index`.
1537+
* @throws Throws an error if the specified range is invalid. In this case, no rows are removed.
14821538
*/
1483-
removeAllColumns(): TreeNodeFromImplicitAllowedTypes<TColumn>[];
1484-
1539+
removeRows(
1540+
index?: number | undefined,
1541+
count?: number | undefined,
1542+
): TreeNodeFromImplicitAllowedTypes<TRow>[];
14851543
/**
14861544
* Removes 0 or more rows from the table.
14871545
* @param rows - The rows to remove.
@@ -1499,12 +1557,6 @@ export namespace TableSchema {
14991557
*/
15001558
removeRows(rows: readonly string[]): TreeNodeFromImplicitAllowedTypes<TRow>[];
15011559

1502-
/**
1503-
* Removes all rows from the table.
1504-
* @returns The removed rows.
1505-
*/
1506-
removeAllRows(): TreeNodeFromImplicitAllowedTypes<TRow>[];
1507-
15081560
/**
15091561
* Removes the cell at the specified location in the table.
15101562
* @returns The cell if it exists, otherwise undefined.

0 commit comments

Comments
 (0)