Skip to content

Commit

Permalink
[TreeView] Throw an error when two items have the same id (#11715)
Browse files Browse the repository at this point in the history
  • Loading branch information
flaviendelangle authored Jan 25, 2024
1 parent 25e8be7 commit 58c5fe8
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,19 @@ export const useTreeViewJSXNodes: TreeViewPlugin<UseTreeViewJSXNodesSignature> =
setState,
}) => {
const insertJSXNode = useEventCallback((node: TreeViewNode) => {
setState((prevState) => ({ ...prevState, nodeMap: { ...prevState.nodeMap, [node.id]: node } }));
setState((prevState) => {
if (prevState.nodeMap[node.id] != null) {
throw new Error(
[
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'Alternatively, you can use the `getItemId` prop to specify a custom id for each item.',
`Tow items were provided with the same id in the \`items\` prop: "${node.id}"`,
].join('\n'),
);
}

return { ...prevState, nodeMap: { ...prevState.nodeMap, [node.id]: node } };
});
});

const removeJSXNode = useEventCallback((nodeId: string) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import * as React from 'react';
import { expect } from 'chai';
import { createRenderer, ErrorBoundary } from '@mui-internal/test-utils';
import { RichTreeView } from '@mui/x-tree-view/RichTreeView';
import { SimpleTreeView } from '@mui/x-tree-view/SimpleTreeView';
import { TreeItem } from '@mui/x-tree-view/TreeItem';

describe('useTreeViewNodes', () => {
const { render } = createRenderer();

it('should throw an error when two items have the same ID (items prop approach)', function test() {
// TODO is this fixed?
if (!/jsdom/.test(window.navigator.userAgent)) {
// can't catch render errors in the browser for unknown reason
// tried try-catch + error boundary + window onError preventDefault
this.skip();
}

expect(() =>
render(
<ErrorBoundary>
<RichTreeView
items={[
{ id: '1', label: '1' },
{ id: '1', label: 'B' },
]}
/>
</ErrorBoundary>,
),
).toErrorDev([
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'The above error occurred in the <ForwardRef(RichTreeView)> component:',
]);
});

it('should throw an error when two items have the same ID (JSX approach)', function test() {
// TODO is this fixed?
if (!/jsdom/.test(window.navigator.userAgent)) {
// can't catch render errors in the browser for unknown reason
// tried try-catch + error boundary + window onError preventDefault
this.skip();
}

expect(() =>
render(
<ErrorBoundary>
<SimpleTreeView>
<TreeItem nodeId="1" label="A" />
<TreeItem nodeId="1" label="B" />
</SimpleTreeView>
</ErrorBoundary>,
),
).toErrorDev([
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'The above error occurred in the <ForwardRef(SimpleTreeView)> component:',
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const updateState = ({
parentId: string | null,
): TreeViewNodeIdAndChildren => {
const id: string = getItemId ? getItemId(item) : (item as any).id;

if (id == null) {
throw new Error(
[
Expand All @@ -40,6 +41,16 @@ const updateState = ({
);
}

if (nodeMap[id] != null) {
throw new Error(
[
'MUI X: The Tree View component requires all items to have a unique `id` property.',
'Alternatively, you can use the `getItemId` prop to specify a custom id for each item.',
`Tow items were provided with the same id in the \`items\` prop: "${id}"`,
].join('\n'),
);
}

const label = getItemLabel ? getItemLabel(item) : (item as { label: string }).label;
if (label == null) {
throw new Error(
Expand Down

0 comments on commit 58c5fe8

Please sign in to comment.