From 4baf116364b47f8286844f759cbb804bc8b8bf0c Mon Sep 17 00:00:00 2001 From: damien Date: Thu, 24 Sep 2020 16:56:16 +0200 Subject: [PATCH 1/5] remove the need for a row id --- packages/grid/_modules_/grid/hooks/root/useRows.ts | 8 ++++++-- packages/grid/_modules_/grid/models/rows.ts | 4 ++-- .../storybook/src/stories/grid-sorting.stories.tsx | 12 ++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/root/useRows.ts b/packages/grid/_modules_/grid/hooks/root/useRows.ts index 13cee8ceb8190..8cd0703afd835 100644 --- a/packages/grid/_modules_/grid/hooks/root/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/root/useRows.ts @@ -30,7 +30,7 @@ export const useRows = ( apiRef: ApiRef, ): RowModel[] => { const logger = useLogger('useRows'); - const rowModels = React.useMemo(() => rows.map((r) => createRow(r)), [rows]); + const rowModels = React.useMemo(() => rows.map((r, idx) => createRow(r, idx)), [rows]); const [rowModelsState, setRowModelsState] = React.useState(rowModels); const [, forceUpdate] = React.useState(); const [rafUpdate] = useRafUpdate(() => forceUpdate((p: any) => !p)); @@ -119,6 +119,10 @@ export const useRows = ( // we removes duplicate updates. A server can batch updates, and send several updates for the same row in one fn call. const uniqUpdates = updates.reduce((uniq, update) => { + if(update.id == null) { + throw new Error('Material-UI: Missing row Id in row data update.'); + } + uniq[update.id] = uniq[update.id] != null ? { ...uniq[update.id], ...update } : update; return uniq; }, {} as { [id: string]: any }); @@ -126,7 +130,7 @@ export const useRows = ( const rowModelUpdates = Object.values(uniqUpdates).map((partialRow) => { const oldRow = getRowFromId(partialRow.id!); if (!oldRow) { - return createRow(partialRow); + return createRow(partialRow, partialRow.id!); } return { ...oldRow, data: { ...oldRow.data, ...partialRow } }; }); diff --git a/packages/grid/_modules_/grid/models/rows.ts b/packages/grid/_modules_/grid/models/rows.ts index 3bf16610891c0..c0f686fc2bdf3 100644 --- a/packages/grid/_modules_/grid/models/rows.ts +++ b/packages/grid/_modules_/grid/models/rows.ts @@ -44,9 +44,9 @@ export interface RowModel { * @param rowData Row as [[RowData]]. * @returns A row as [[RowModel]]. */ -export function createRow(rowData: RowData): RowModel { +export function createRow(rowData: RowData, index: RowId): RowModel { const row: RowModel = { - id: rowData.id, + id: rowData.id == null ? index : rowData.id, data: rowData, selected: false, }; diff --git a/packages/storybook/src/stories/grid-sorting.stories.tsx b/packages/storybook/src/stories/grid-sorting.stories.tsx index 13208c4f6a879..8f6ea55ee60f1 100644 --- a/packages/storybook/src/stories/grid-sorting.stories.tsx +++ b/packages/storybook/src/stories/grid-sorting.stories.tsx @@ -248,6 +248,18 @@ export const SortingWithFormatter = () => { ); }; +export const SortingNoId = () => { + const columns = [...getColumns()]; + columns.shift(); + columns[0] = { ...columns[0], sortDirection: 'asc' }; + + return ( +
+ +
+ ); +}; + export const SortModelOptionsMultiple = () => { const sortModel: SortModel = React.useMemo( () => [ From cb6b701e19d83e343f55cbbb8b7696daef26c149 Mon Sep 17 00:00:00 2001 From: damien Date: Thu, 24 Sep 2020 18:15:08 +0200 Subject: [PATCH 2/5] fix prettier --- packages/grid/_modules_/grid/hooks/root/useRows.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/hooks/root/useRows.ts b/packages/grid/_modules_/grid/hooks/root/useRows.ts index 8cd0703afd835..4d81515e5fb54 100644 --- a/packages/grid/_modules_/grid/hooks/root/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/root/useRows.ts @@ -119,7 +119,7 @@ export const useRows = ( // we removes duplicate updates. A server can batch updates, and send several updates for the same row in one fn call. const uniqUpdates = updates.reduce((uniq, update) => { - if(update.id == null) { + if (update.id == null) { throw new Error('Material-UI: Missing row Id in row data update.'); } From 6b62e3bed7a67da4dedbaf4f969e951495b78335 Mon Sep 17 00:00:00 2001 From: damien Date: Fri, 25 Sep 2020 11:38:29 +0200 Subject: [PATCH 3/5] Rename createRow to createRowModel --- packages/grid/_modules_/grid/hooks/root/useRows.ts | 6 +++--- packages/grid/_modules_/grid/models/rows.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/root/useRows.ts b/packages/grid/_modules_/grid/hooks/root/useRows.ts index 4d81515e5fb54..6f49391534fa1 100644 --- a/packages/grid/_modules_/grid/hooks/root/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/root/useRows.ts @@ -1,6 +1,6 @@ import * as React from 'react'; import { - createRow, + createRowModel, GridOptions, RowData, RowId, @@ -30,7 +30,7 @@ export const useRows = ( apiRef: ApiRef, ): RowModel[] => { const logger = useLogger('useRows'); - const rowModels = React.useMemo(() => rows.map((r, idx) => createRow(r, idx)), [rows]); + const rowModels = React.useMemo(() => rows.map((r, idx) => createRowModel(r, idx)), [rows]); const [rowModelsState, setRowModelsState] = React.useState(rowModels); const [, forceUpdate] = React.useState(); const [rafUpdate] = useRafUpdate(() => forceUpdate((p: any) => !p)); @@ -130,7 +130,7 @@ export const useRows = ( const rowModelUpdates = Object.values(uniqUpdates).map((partialRow) => { const oldRow = getRowFromId(partialRow.id!); if (!oldRow) { - return createRow(partialRow, partialRow.id!); + return createRowModel(partialRow, partialRow.id!); } return { ...oldRow, data: { ...oldRow.data, ...partialRow } }; }); diff --git a/packages/grid/_modules_/grid/models/rows.ts b/packages/grid/_modules_/grid/models/rows.ts index c0f686fc2bdf3..be2fc956d5899 100644 --- a/packages/grid/_modules_/grid/models/rows.ts +++ b/packages/grid/_modules_/grid/models/rows.ts @@ -44,7 +44,7 @@ export interface RowModel { * @param rowData Row as [[RowData]]. * @returns A row as [[RowModel]]. */ -export function createRow(rowData: RowData, index: RowId): RowModel { +export function createRowModel(rowData: RowData, index: RowId): RowModel { const row: RowModel = { id: rowData.id == null ? index : rowData.id, data: rowData, From 86f490bb73f618d418c326498525323210ccc8ae Mon Sep 17 00:00:00 2001 From: damien Date: Fri, 25 Sep 2020 14:44:06 +0200 Subject: [PATCH 4/5] throw error when row has no id --- packages/grid/_modules_/grid/hooks/root/useRows.ts | 4 ++-- packages/grid/_modules_/grid/models/rows.ts | 8 ++++++-- .../storybook/src/stories/grid-sorting.stories.tsx | 11 ----------- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/root/useRows.ts b/packages/grid/_modules_/grid/hooks/root/useRows.ts index 6f49391534fa1..5f4243dd004d6 100644 --- a/packages/grid/_modules_/grid/hooks/root/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/root/useRows.ts @@ -30,7 +30,7 @@ export const useRows = ( apiRef: ApiRef, ): RowModel[] => { const logger = useLogger('useRows'); - const rowModels = React.useMemo(() => rows.map((r, idx) => createRowModel(r, idx)), [rows]); + const rowModels = React.useMemo(() => rows.map((r) => createRowModel(r)), [rows]); const [rowModelsState, setRowModelsState] = React.useState(rowModels); const [, forceUpdate] = React.useState(); const [rafUpdate] = useRafUpdate(() => forceUpdate((p: any) => !p)); @@ -130,7 +130,7 @@ export const useRows = ( const rowModelUpdates = Object.values(uniqUpdates).map((partialRow) => { const oldRow = getRowFromId(partialRow.id!); if (!oldRow) { - return createRowModel(partialRow, partialRow.id!); + return createRowModel(partialRow); } return { ...oldRow, data: { ...oldRow.data, ...partialRow } }; }); diff --git a/packages/grid/_modules_/grid/models/rows.ts b/packages/grid/_modules_/grid/models/rows.ts index be2fc956d5899..e9daab6edd39e 100644 --- a/packages/grid/_modules_/grid/models/rows.ts +++ b/packages/grid/_modules_/grid/models/rows.ts @@ -44,9 +44,13 @@ export interface RowModel { * @param rowData Row as [[RowData]]. * @returns A row as [[RowModel]]. */ -export function createRowModel(rowData: RowData, index: RowId): RowModel { +export function createRowModel(rowData: RowData): RowModel { + if (rowData.id == null) { + throw new Error(`Material-UI: Row without id found, `); + } + const row: RowModel = { - id: rowData.id == null ? index : rowData.id, + id: rowData.id, data: rowData, selected: false, }; diff --git a/packages/storybook/src/stories/grid-sorting.stories.tsx b/packages/storybook/src/stories/grid-sorting.stories.tsx index 8f6ea55ee60f1..8d9b4e489356d 100644 --- a/packages/storybook/src/stories/grid-sorting.stories.tsx +++ b/packages/storybook/src/stories/grid-sorting.stories.tsx @@ -248,17 +248,6 @@ export const SortingWithFormatter = () => { ); }; -export const SortingNoId = () => { - const columns = [...getColumns()]; - columns.shift(); - columns[0] = { ...columns[0], sortDirection: 'asc' }; - - return ( -
- -
- ); -}; export const SortModelOptionsMultiple = () => { const sortModel: SortModel = React.useMemo( From 25e8e99979fe257763f2ddc7daa081ed7c4ac897 Mon Sep 17 00:00:00 2001 From: damien Date: Fri, 25 Sep 2020 15:12:48 +0200 Subject: [PATCH 5/5] fix error message and added test --- .../grid/_modules_/grid/hooks/root/useRows.ts | 8 +++++++- .../_modules_/grid/models/__tests__/rows.tests.ts | 15 +++++++++++++++ packages/grid/_modules_/grid/models/rows.ts | 8 +++++++- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 packages/grid/_modules_/grid/models/__tests__/rows.tests.ts diff --git a/packages/grid/_modules_/grid/hooks/root/useRows.ts b/packages/grid/_modules_/grid/hooks/root/useRows.ts index 5f4243dd004d6..cd2eaeecbe94e 100644 --- a/packages/grid/_modules_/grid/hooks/root/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/root/useRows.ts @@ -120,7 +120,13 @@ export const useRows = ( // we removes duplicate updates. A server can batch updates, and send several updates for the same row in one fn call. const uniqUpdates = updates.reduce((uniq, update) => { if (update.id == null) { - throw new Error('Material-UI: Missing row Id in row data update.'); + throw new Error( + [ + 'Material-UI: The data grid component requires all rows to have a unique id property.', + 'A row was provided without when calling updateRowData():', + JSON.stringify(update), + ].join('\n'), + ); } uniq[update.id] = uniq[update.id] != null ? { ...uniq[update.id], ...update } : update; diff --git a/packages/grid/_modules_/grid/models/__tests__/rows.tests.ts b/packages/grid/_modules_/grid/models/__tests__/rows.tests.ts new file mode 100644 index 0000000000000..c4ad5f78aea30 --- /dev/null +++ b/packages/grid/_modules_/grid/models/__tests__/rows.tests.ts @@ -0,0 +1,15 @@ +import { createRowModel, RowData } from '../rows'; + +describe('Row: createRowModel', () => { + it('should have an id', () => { + const row = { + name: 'damien', + }; + const expectedError = [ + 'Material-UI: The data grid component requires all rows to have a unique id property.', + 'A row was provided without in the rows prop:', + JSON.stringify(row), + ].join('\n'); + expect(() => createRowModel((row))).toThrow(expectedError); + }); +}); diff --git a/packages/grid/_modules_/grid/models/rows.ts b/packages/grid/_modules_/grid/models/rows.ts index e9daab6edd39e..854056476d398 100644 --- a/packages/grid/_modules_/grid/models/rows.ts +++ b/packages/grid/_modules_/grid/models/rows.ts @@ -46,7 +46,13 @@ export interface RowModel { */ export function createRowModel(rowData: RowData): RowModel { if (rowData.id == null) { - throw new Error(`Material-UI: Row without id found, `); + throw new Error( + [ + 'Material-UI: The data grid component requires all rows to have a unique id property.', + 'A row was provided without in the rows prop:', + JSON.stringify(rowData), + ].join('\n'), + ); } const row: RowModel = {