From 29d3b79a0b37032540f1bec3abe9f8daac18b40e Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 25 Apr 2023 06:33:16 -0700 Subject: [PATCH] DataGrid: Avoid MutableDataFrame where possible (#67174) * avoid mutable * remove mutableDataFrame from utils also * fix build and add couple more testss --------- Co-authored-by: Victor Marin --- .../plugins/panel/datagrid/DataGridPanel.tsx | 75 +++++++++---------- .../app/plugins/panel/datagrid/utils.test.ts | 27 +++++++ public/app/plugins/panel/datagrid/utils.ts | 23 +++--- 3 files changed, 75 insertions(+), 50 deletions(-) diff --git a/public/app/plugins/panel/datagrid/DataGridPanel.tsx b/public/app/plugins/panel/datagrid/DataGridPanel.tsx index 7e0ba377af4..f99370c49ad 100644 --- a/public/app/plugins/panel/datagrid/DataGridPanel.tsx +++ b/public/app/plugins/panel/datagrid/DataGridPanel.tsx @@ -10,7 +10,7 @@ import DataEditor, { } from '@glideapps/glide-data-grid'; import React, { useEffect, useReducer } from 'react'; -import { ArrayVector, Field, MutableDataFrame, PanelProps, FieldType } from '@grafana/data'; +import { Field, PanelProps, FieldType } from '@grafana/data'; import { PanelDataErrorView } from '@grafana/runtime'; import { useTheme2 } from '@grafana/ui'; @@ -85,36 +85,37 @@ export function DataGridPanel({ options, data, id, fieldConfig, width, height }: const values = field.values.toArray(); values[row] = newValue.data; - field.values = new ArrayVector(values); + field.values = values; - publishSnapshot(new MutableDataFrame(frame), id); + publishSnapshot(frame, id); }; const onColumnInputBlur = (columnName: string) => { const len = frame.length ?? 0; - const newFrame = new MutableDataFrame(frame); - - const field: Field = { - name: columnName, - type: FieldType.string, - config: {}, - values: new ArrayVector(new Array(len).fill('')), - }; - - newFrame.addField(field); - - publishSnapshot(newFrame, id); + publishSnapshot( + { + ...frame, + fields: [ + ...frame.fields, + { + name: columnName, + type: FieldType.string, + config: {}, + values: new Array(len).fill(''), + }, + ], + }, + id + ); }; const addNewRow = () => { - //TODO use .appendRow() after fieldValues refactor is finished - const newFrame = new MutableDataFrame(frame); - - newFrame.fields.map((field) => { - field.values = new ArrayVector([...field.values.toArray(), null]); + const fields = frame.fields.map((f) => { + const values = f.values.slice(); // copy + values.push(null); + return { ...f, values }; }); - - publishSnapshot(newFrame, id); + publishSnapshot({ ...frame, fields, length: frame.length + 1 }, id); }; const onColumnResize = (column: GridColumn, width: number, columnIndex: number, newSizeWithGrow: number) => { @@ -162,27 +163,25 @@ export function DataGridPanel({ options, data, id, fieldConfig, width, height }: }; const onColumnMove = (from: number, to: number) => { - const newFrame = new MutableDataFrame(frame); - const field = newFrame.fields[from]; - newFrame.fields.splice(from, 1); - newFrame.fields.splice(to, 0, field); + const fields = frame.fields.map((f) => f); + const field = fields[from]; + fields.splice(from, 1); + fields.splice(to, 0, field); dispatch({ type: DatagridActionType.columnMove, payload: { from, to } }); - publishSnapshot(newFrame, id); + publishSnapshot({ ...frame, fields }, id); }; const onRowMove = (from: number, to: number) => { - const newFrame = new MutableDataFrame(frame); + const fields = frame.fields.map((f) => ({ ...f, values: f.values.slice() })); - for (const field of newFrame.fields) { - const values = field.values.toArray(); - const value = values[from]; - values.splice(from, 1); - values.splice(to, 0, value); - field.values = new ArrayVector(values); + for (const field of fields) { + const value = field.values[from]; + field.values.splice(from, 1); + field.values.splice(to, 0, value); } - publishSnapshot(newFrame, id); + publishSnapshot({ ...frame, fields }, id); }; const onColumnRename = () => { @@ -190,11 +189,11 @@ export function DataGridPanel({ options, data, id, fieldConfig, width, height }: }; const onRenameInputBlur = (columnName: string, columnIdx: number) => { - const newFrame = new MutableDataFrame(frame); - newFrame.fields[columnIdx].name = columnName; + const fields = frame.fields.map((f) => f); + fields[columnIdx].name = columnName; dispatch({ type: DatagridActionType.hideColumnRenameInput }); - publishSnapshot(newFrame, id); + publishSnapshot({ ...frame, fields }, id); }; const onSearchClose = () => { diff --git a/public/app/plugins/panel/datagrid/utils.test.ts b/public/app/plugins/panel/datagrid/utils.test.ts index 9face79dd1f..3d493c2106a 100644 --- a/public/app/plugins/panel/datagrid/utils.test.ts +++ b/public/app/plugins/panel/datagrid/utils.test.ts @@ -70,6 +70,20 @@ describe('when deleting rows', () => { expect(newDf.fields[2].values.toArray()).toEqual([]); expect(newDf.length).toEqual(0); }); + + it('should do nothing if there are no fields', () => { + const newDf = deleteRows( + { + name: 'emptyDataframe', + fields: [], + length: 0, + }, + [0, 1, 2, 3, 4], + true + ); + + expect(newDf.length).toEqual(0); + }); }); describe('when clearing cells from range selection', () => { @@ -119,4 +133,17 @@ describe('when clearing cells from range selection', () => { expect(newDf.fields[2].values.toArray()).toEqual(['a', 'b', 'c', 'd', 'e']); expect(newDf.length).toEqual(5); }); + + it('should do nothing if there are no fields', () => { + const newDf = clearCellsFromRangeSelection( + { + name: 'emptyDataframe', + fields: [], + length: 0, + }, + { x: 0, y: 0, width: 0, height: 0 } + ); + + expect(newDf.length).toEqual(0); + }); }); diff --git a/public/app/plugins/panel/datagrid/utils.ts b/public/app/plugins/panel/datagrid/utils.ts index 993cecc1798..5cb6cf08724 100644 --- a/public/app/plugins/panel/datagrid/utils.ts +++ b/public/app/plugins/panel/datagrid/utils.ts @@ -1,16 +1,7 @@ import { css } from '@emotion/css'; import { CompactSelection, GridCell, GridCellKind, GridSelection, Theme } from '@glideapps/glide-data-grid'; -import { - ArrayVector, - DataFrame, - DataFrameJSON, - dataFrameToJSON, - MutableDataFrame, - Field, - GrafanaTheme2, - FieldType, -} from '@grafana/data'; +import { ArrayVector, DataFrame, DataFrameJSON, dataFrameToJSON, Field, GrafanaTheme2, FieldType } from '@grafana/data'; import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; import { GrafanaQuery, GrafanaQueryType } from 'app/plugins/datasource/grafana/types'; @@ -131,7 +122,11 @@ export const deleteRows = (gridData: DataFrame, rows: number[], hardDelete = fal field.values = new ArrayVector(valuesArray); } - return new MutableDataFrame(gridData); + return { + ...gridData, + fields: [...gridData.fields], + length: gridData.fields[0]?.values.length ?? 0, + }; }; export const clearCellsFromRangeSelection = (gridData: DataFrame, range: CellRange): DataFrame => { @@ -147,7 +142,11 @@ export const clearCellsFromRangeSelection = (gridData: DataFrame, range: CellRan field.values = new ArrayVector(valuesArray); } - return new MutableDataFrame(gridData); + return { + ...gridData, + fields: [...gridData.fields], + length: gridData.fields[0]?.values.length ?? 0, + }; }; export const publishSnapshot = (data: DataFrame, panelID: number): void => {