From c9e7d1351ba40569946431d981ef5a3e5340926d Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 13:22:13 -0800 Subject: [PATCH 01/12] Pure render for useTreeData --- package.json | 4 +- .../listbox/stories/ListBox.stories.tsx | 1 + .../listbox/test/ListBox.test.js | 4 +- .../@react-stately/data/src/useTreeData.ts | 52 ++++++++++--------- yarn.lock | 22 ++++++++ 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index 4032e8ec4c4..e083518cf6b 100644 --- a/package.json +++ b/package.json @@ -157,9 +157,9 @@ "postcss-import": "^15.1.0", "prop-types": "^15.6.0", "raf": "^3.4.0", - "react": "^18.0.0", + "react": "^18.3.0-canary-2cd19ed1d-20240207", "react-axe": "^3.0.2", - "react-dom": "^18.0.0", + "react-dom": "^18.3.0-canary-2cd19ed1d-20240207", "react-test-renderer": "^16.9.0", "recast": "^0.20", "recursive-readdir": "^2.2.2", diff --git a/packages/@react-spectrum/listbox/stories/ListBox.stories.tsx b/packages/@react-spectrum/listbox/stories/ListBox.stories.tsx index a0d154a4281..e3e09a353e0 100644 --- a/packages/@react-spectrum/listbox/stories/ListBox.stories.tsx +++ b/packages/@react-spectrum/listbox/stories/ListBox.stories.tsx @@ -940,6 +940,7 @@ export function FocusExample(args = {}) { getKey: (item) => item.name, getChildren: (item:{name:string, children?:{name:string, children?:{name:string}[]}[]}) => item.children }); + let [dialog, setDialog] = useState(null); let ref = useRef(null); return ( diff --git a/packages/@react-spectrum/listbox/test/ListBox.test.js b/packages/@react-spectrum/listbox/test/ListBox.test.js index f60ea193012..c726cc129c3 100644 --- a/packages/@react-spectrum/listbox/test/ListBox.test.js +++ b/packages/@react-spectrum/listbox/test/ListBox.test.js @@ -889,8 +889,9 @@ describe('ListBox', function () { }); describe('When focused item is removed', function () { - it('should move focus to the next item that is not disabled', () => { + it.only('should move focus to the next item that is not disabled', () => { let tree = render(); + console.log('done render') act(() => jest.runAllTimers()); let listbox = tree.getByRole('listbox'); let options = within(listbox).getAllByRole('option'); @@ -910,6 +911,7 @@ describe('ListBox', function () { expect(document.activeElement).toBe(confirmationDialog); let confirmationDialogButton = within(confirmationDialog).getByRole('button'); expect(confirmationDialogButton).toBeInTheDocument(); + console.log('deleting') triggerPress(confirmationDialogButton); act(() => jest.runAllTimers()); options = within(listbox).getAllByRole('option'); diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 09a846d1a05..f7e24114a50 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -114,7 +114,7 @@ export interface TreeData { */ update(key: Key, newValue: T): void } - +let creation = 0; /** * Manages state for an immutable tree data structure, and provides convenience methods to * update the data over time. @@ -126,17 +126,18 @@ export function useTreeData(options: TreeOptions): TreeData getKey = (item: any) => item.id || item.key, getChildren = (item: any) => item.children } = options; - let map = useMemo(() => new Map>(), []); // We only want to compute this on initial render. - // eslint-disable-next-line react-hooks/exhaustive-deps - let initialNodes = useMemo(() => buildTree(initialItems), []); - let [items, setItems] = useState(initialNodes); + let [tree, setItems] = useState(() => buildTree(initialItems)); + let [items, map] = tree; + let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); function buildTree(initialItems: T[] = [], parentKey?: Key | null) { - return initialItems.map(item => { + let map = new Map>(); + return [initialItems.map(item => { let node: TreeNode = { + creation: creation++, key: getKey(item), parentKey: parentKey, value: item, @@ -146,14 +147,15 @@ export function useTreeData(options: TreeOptions): TreeData node.children = buildTree(getChildren(item), node.key); map.set(node.key, node); return node; - }); + }), map]; } - function updateTree(items: TreeNode[], key: Key, update: (node: TreeNode) => TreeNode) { - let node = map.get(key); + function updateTree(items: TreeNode[], key: Key, update: (node: TreeNode) => TreeNode, originalMap: Map>) { + let node = originalMap.get(key); if (!node) { - return items; + return [items, originalMap]; } + let map = new Map>(originalMap); // Create a new node. If null, then delete the node, otherwise replace. let newNode = update(node); @@ -167,6 +169,7 @@ export function useTreeData(options: TreeOptions): TreeData while (node.parentKey) { let nextParent = map.get(node.parentKey); let copy: TreeNode = { + creation: creation++, key: nextParent.key, parentKey: nextParent.parentKey, value: nextParent.value, @@ -196,13 +199,14 @@ export function useTreeData(options: TreeOptions): TreeData items = items.filter(c => c !== node); } - return items.map(item => { + return [items.map(item => { + // if (item.key === node.key) { if (item === node) { return newNode; } return item; - }); + }), map]; } function addNode(node: TreeNode) { @@ -227,16 +231,16 @@ export function useTreeData(options: TreeOptions): TreeData return map.get(key); }, insert(parentKey: Key | null, index: number, ...values: T[]) { - setItems(items => { + setItems(([items, originalMap]) => { let nodes = buildTree(values, parentKey); // If parentKey is null, insert into the root. if (parentKey == null) { - return [ + return [[ ...items.slice(0, index), ...nodes, ...items.slice(index) - ]; + ], originalMap]; } // Otherwise, update the parent node and its ancestors. @@ -249,7 +253,7 @@ export function useTreeData(options: TreeOptions): TreeData ...nodes, ...parentNode.children.slice(index) ] - })); + }), originalMap); }); }, insertBefore(key: Key, ...values: T[]): void { @@ -292,7 +296,7 @@ export function useTreeData(options: TreeOptions): TreeData remove(...keys: Key[]) { let newItems = items; for (let key of keys) { - newItems = updateTree(newItems, key, () => null); + newItems = updateTree(newItems, key, () => null, map); } setItems(newItems); @@ -310,13 +314,13 @@ export function useTreeData(options: TreeOptions): TreeData this.remove(...selectedKeys); }, move(key: Key, toParentKey: Key | null, index: number) { - setItems(items => { + setItems(([items, originalMap]) => { let node = map.get(key); if (!node) { return items; } - items = updateTree(items, key, () => null); + items = updateTree(items, key, () => null, originalMap); const movedNode = { ...node, @@ -325,11 +329,11 @@ export function useTreeData(options: TreeOptions): TreeData // If parentKey is null, insert into the root. if (toParentKey == null) { - return [ + return [[ ...items.slice(0, index), movedNode, ...items.slice(index) - ]; + ], originalMap]; } // Otherwise, update the parent node and its ancestors. @@ -342,11 +346,11 @@ export function useTreeData(options: TreeOptions): TreeData movedNode, ...parentNode.children.slice(index) ] - })); + }), originalMap); }); }, update(oldKey: Key, newValue: T) { - setItems(items => updateTree(items, oldKey, oldNode => { + setItems(([items, originalMap]) => updateTree(items, oldKey, oldNode => { let node: TreeNode = { key: oldNode.key, parentKey: oldNode.parentKey, @@ -356,7 +360,7 @@ export function useTreeData(options: TreeOptions): TreeData node.children = buildTree(getChildren(newValue), node.key); return node; - })); + }, originalMap)); } }; } diff --git a/yarn.lock b/yarn.lock index 44accbaab66..be3f2454ef0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -21117,6 +21117,14 @@ react-dom@^18.0.0: loose-envify "^1.1.0" scheduler "^0.22.0" +react-dom@^18.3.0-canary-2cd19ed1d-20240207: + version "18.3.0-canary-2cd19ed1d-20240207" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.3.0-canary-2cd19ed1d-20240207.tgz#24759a8eb114bd8d3d543af332fb52d0872a7281" + integrity sha512-vo0KNb73J0vEGvmpr6uh+q/9579liEVU9wgVH3KbaZg4+Q7j6jcneXUvwp6Vf4xxLNNOI94CwcDcDvguLm6O6A== + dependencies: + loose-envify "^1.1.0" + scheduler "0.24.0-canary-2cd19ed1d-20240207" + react-element-to-jsx-string@^14.3.4: version "14.3.4" resolved "https://registry.yarnpkg.com/react-element-to-jsx-string/-/react-element-to-jsx-string-14.3.4.tgz#709125bc72f06800b68f9f4db485f2c7d31218a8" @@ -21205,6 +21213,13 @@ react@^18.0.0: dependencies: loose-envify "^1.1.0" +react@^18.3.0-canary-2cd19ed1d-20240207: + version "18.3.0-canary-2cd19ed1d-20240207" + resolved "https://registry.yarnpkg.com/react/-/react-18.3.0-canary-2cd19ed1d-20240207.tgz#9c1f481b71fde2d3ba3dbc38ab96e291bbb5c769" + integrity sha512-Fu00nsyCds3mH713G9/zjif2P3O8j49vNqmv1gMQjYi4SosmhOW2BlLOpOASwmHCpgPspYs8YTCDRAXkT+3o8g== + dependencies: + loose-envify "^1.1.0" + read-cache@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/read-cache/-/read-cache-1.0.0.tgz#e664ef31161166c9751cdbe8dbcf86b5fb58f774" @@ -22140,6 +22155,13 @@ saxes@^6.0.0: dependencies: xmlchars "^2.2.0" +scheduler@0.24.0-canary-2cd19ed1d-20240207: + version "0.24.0-canary-2cd19ed1d-20240207" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.24.0-canary-2cd19ed1d-20240207.tgz#48075b97f1afbea3ed6caf1f70ac72576f25c07f" + integrity sha512-9+0q2o4q0Nhlk7rGJf6sL08+Zp2qilbUkWCMhmDXC/codtdrzE9WqrxakqrpHet7NmZigcVyPKJFSWH6kS+uLw== + dependencies: + loose-envify "^1.1.0" + scheduler@^0.16.2: version "0.16.2" resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.16.2.tgz#f74cd9d33eff6fc554edfb79864868e4819132c1" From ea85e9eb21ba09004e33dfdbf9b9a9748e4ca5f5 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 13:47:46 -0800 Subject: [PATCH 02/12] adjust some more calls --- .../@react-stately/data/src/useTreeData.ts | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index f7e24114a50..6749361e396 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -165,7 +165,7 @@ export function useTreeData(options: TreeOptions): TreeData addNode(newNode); } - // Walk up the tree and update each parent to refer to the new chilren. + // Walk up the tree and update each parent to refer to the new children. while (node.parentKey) { let nextParent = map.get(node.parentKey); let copy: TreeNode = { @@ -295,8 +295,10 @@ export function useTreeData(options: TreeOptions): TreeData }, remove(...keys: Key[]) { let newItems = items; + let prevMap = map; for (let key of keys) { - newItems = updateTree(newItems, key, () => null, map); + newItems = updateTree(newItems, key, () => null, prevMap); + prevMap = newItems[1]; } setItems(newItems); @@ -315,7 +317,7 @@ export function useTreeData(options: TreeOptions): TreeData }, move(key: Key, toParentKey: Key | null, index: number) { setItems(([items, originalMap]) => { - let node = map.get(key); + let node = originalMap.get(key); if (!node) { return items; } @@ -330,14 +332,14 @@ export function useTreeData(options: TreeOptions): TreeData // If parentKey is null, insert into the root. if (toParentKey == null) { return [[ - ...items.slice(0, index), + ...items[0].slice(0, index), movedNode, - ...items.slice(index) - ], originalMap]; + ...items[0].slice(index) + ], items[1]]; } // Otherwise, update the parent node and its ancestors. - return updateTree(items, toParentKey, parentNode => ({ + return updateTree(items[0], toParentKey, parentNode => ({ key: parentNode.key, parentKey: parentNode.parentKey, value: parentNode.value, @@ -346,7 +348,7 @@ export function useTreeData(options: TreeOptions): TreeData movedNode, ...parentNode.children.slice(index) ] - }), originalMap); + }), items[1]); }); }, update(oldKey: Key, newValue: T) { @@ -358,8 +360,9 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - node.children = buildTree(getChildren(newValue), node.key); - return node; + let tree = buildTree(getChildren(newValue), node.key); + node.children = tree[0]; + return [node, tree[1]]; }, originalMap)); } }; From 025705476f27e54f0b525f53b0e55a7e2b032956 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 13:52:59 -0800 Subject: [PATCH 03/12] fix accumulation of build trees --- packages/@react-stately/data/src/useTreeData.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 6749361e396..1b8a05cea6e 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -133,8 +133,11 @@ export function useTreeData(options: TreeOptions): TreeData let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); - function buildTree(initialItems: T[] = [], parentKey?: Key | null) { - let map = new Map>(); + function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc: Map>) { + let map = acc; + if (!acc) { + map = new Map>(); + } return [initialItems.map(item => { let node: TreeNode = { creation: creation++, @@ -144,7 +147,7 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - node.children = buildTree(getChildren(item), node.key); + node.children = buildTree(getChildren(item), node.key, map)[0]; map.set(node.key, node); return node; }), map]; From 86d3a0f9c920fde5ad1683205a2d7142443aadf1 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 14:25:36 -0800 Subject: [PATCH 04/12] better readability --- .../@react-stately/data/src/useTreeData.ts | 102 ++++++++++-------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 1b8a05cea6e..beacbf9f4e6 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -128,35 +128,38 @@ export function useTreeData(options: TreeOptions): TreeData } = options; // We only want to compute this on initial render. - let [tree, setItems] = useState(() => buildTree(initialItems)); - let [items, map] = tree; + let [tree, setItems] = useState<{items: TreeNode[], mappy: Map>}>(() => buildTree(initialItems)); + let {items, mappy: map} = tree; let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); - function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc: Map>) { + function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc?: Map>) { let map = acc; if (!acc) { map = new Map>(); } - return [initialItems.map(item => { - let node: TreeNode = { - creation: creation++, - key: getKey(item), - parentKey: parentKey, - value: item, - children: null - }; + return { + items: initialItems.map(item => { + let node: TreeNode = { + creation: creation++, + key: getKey(item), + parentKey: parentKey, + value: item, + children: null + }; - node.children = buildTree(getChildren(item), node.key, map)[0]; - map.set(node.key, node); - return node; - }), map]; + node.children = buildTree(getChildren(item), node.key, map)[0]; + map.set(node.key, node); + return node; + }), + mappy: map + }; } function updateTree(items: TreeNode[], key: Key, update: (node: TreeNode) => TreeNode, originalMap: Map>) { let node = originalMap.get(key); if (!node) { - return [items, originalMap]; + return {items, mappy: originalMap}; } let map = new Map>(originalMap); @@ -202,14 +205,17 @@ export function useTreeData(options: TreeOptions): TreeData items = items.filter(c => c !== node); } - return [items.map(item => { - // if (item.key === node.key) { - if (item === node) { - return newNode; - } + return { + items: items.map(item => { + // if (item.key === node.key) { + if (item === node) { + return newNode; + } - return item; - }), map]; + return item; + }), + mappy: map + }; } function addNode(node: TreeNode) { @@ -234,16 +240,19 @@ export function useTreeData(options: TreeOptions): TreeData return map.get(key); }, insert(parentKey: Key | null, index: number, ...values: T[]) { - setItems(([items, originalMap]) => { - let nodes = buildTree(values, parentKey); + setItems(({items, mappy: originalMap}) => { + let {items: nodes, mappy: newMap} = buildTree(values, parentKey); // If parentKey is null, insert into the root. if (parentKey == null) { - return [[ - ...items.slice(0, index), - ...nodes, - ...items.slice(index) - ], originalMap]; + return { + items: [ + ...items.slice(0, index), + ...nodes, + ...items.slice(index) + ], + mappy: newMap + }; } // Otherwise, update the parent node and its ancestors. @@ -256,7 +265,7 @@ export function useTreeData(options: TreeOptions): TreeData ...nodes, ...parentNode.children.slice(index) ] - }), originalMap); + }), newMap); }); }, insertBefore(key: Key, ...values: T[]): void { @@ -300,11 +309,11 @@ export function useTreeData(options: TreeOptions): TreeData let newItems = items; let prevMap = map; for (let key of keys) { - newItems = updateTree(newItems, key, () => null, prevMap); - prevMap = newItems[1]; + let tree = updateTree(newItems, key, () => null, prevMap); + prevMap = tree.mappy; } - setItems(newItems); + setItems(tree); let selection = new Set(selectedKeys); for (let key of selectedKeys) { @@ -319,13 +328,14 @@ export function useTreeData(options: TreeOptions): TreeData this.remove(...selectedKeys); }, move(key: Key, toParentKey: Key | null, index: number) { - setItems(([items, originalMap]) => { + setItems(({items, mappy: originalMap}) => { let node = originalMap.get(key); if (!node) { - return items; + return {items, mappy: originalMap}; } - items = updateTree(items, key, () => null, originalMap); + let tree = updateTree(items, key, () => null, originalMap); + let {items: newItems, mappy: map} = tree; const movedNode = { ...node, @@ -334,15 +344,15 @@ export function useTreeData(options: TreeOptions): TreeData // If parentKey is null, insert into the root. if (toParentKey == null) { - return [[ - ...items[0].slice(0, index), + return {items: [ + ...newItems.slice(0, index), movedNode, - ...items[0].slice(index) - ], items[1]]; + ...newItems.slice(index) + ], mappy: map}; } // Otherwise, update the parent node and its ancestors. - return updateTree(items[0], toParentKey, parentNode => ({ + return updateTree(newItems, toParentKey, parentNode => ({ key: parentNode.key, parentKey: parentNode.parentKey, value: parentNode.value, @@ -351,11 +361,11 @@ export function useTreeData(options: TreeOptions): TreeData movedNode, ...parentNode.children.slice(index) ] - }), items[1]); + }), map); }); }, update(oldKey: Key, newValue: T) { - setItems(([items, originalMap]) => updateTree(items, oldKey, oldNode => { + setItems(({items, mappy: originalMap}) => updateTree(items, oldKey, oldNode => { let node: TreeNode = { key: oldNode.key, parentKey: oldNode.parentKey, @@ -364,8 +374,8 @@ export function useTreeData(options: TreeOptions): TreeData }; let tree = buildTree(getChildren(newValue), node.key); - node.children = tree[0]; - return [node, tree[1]]; + node.children = tree.items; + return node; }, originalMap)); } }; From 62a7fef47fa349b3dbdcf14b5d906ddc799eaba8 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 15:18:35 -0800 Subject: [PATCH 05/12] fix typo --- packages/@react-stately/data/src/useTreeData.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index beacbf9f4e6..3a8672ffde3 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -148,7 +148,7 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - node.children = buildTree(getChildren(item), node.key, map)[0]; + node.children = buildTree(getChildren(item), node.key, map).items; map.set(node.key, node); return node; }), From ca1545a66450f256aa217ad11870d64b32dccd86 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 15:59:47 -0800 Subject: [PATCH 06/12] fix remaining broken tests --- .../listbox/test/ListBox.test.js | 4 +-- .../@react-stately/data/src/useTreeData.ts | 26 +++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/@react-spectrum/listbox/test/ListBox.test.js b/packages/@react-spectrum/listbox/test/ListBox.test.js index c726cc129c3..f60ea193012 100644 --- a/packages/@react-spectrum/listbox/test/ListBox.test.js +++ b/packages/@react-spectrum/listbox/test/ListBox.test.js @@ -889,9 +889,8 @@ describe('ListBox', function () { }); describe('When focused item is removed', function () { - it.only('should move focus to the next item that is not disabled', () => { + it('should move focus to the next item that is not disabled', () => { let tree = render(); - console.log('done render') act(() => jest.runAllTimers()); let listbox = tree.getByRole('listbox'); let options = within(listbox).getAllByRole('option'); @@ -911,7 +910,6 @@ describe('ListBox', function () { expect(document.activeElement).toBe(confirmationDialog); let confirmationDialogButton = within(confirmationDialog).getByRole('button'); expect(confirmationDialogButton).toBeInTheDocument(); - console.log('deleting') triggerPress(confirmationDialogButton); act(() => jest.runAllTimers()); options = within(listbox).getAllByRole('option'); diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 3a8672ffde3..7330a2c537e 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -114,7 +114,7 @@ export interface TreeData { */ update(key: Key, newValue: T): void } -let creation = 0; + /** * Manages state for an immutable tree data structure, and provides convenience methods to * update the data over time. @@ -141,7 +141,6 @@ export function useTreeData(options: TreeOptions): TreeData return { items: initialItems.map(item => { let node: TreeNode = { - creation: creation++, key: getKey(item), parentKey: parentKey, value: item, @@ -166,16 +165,15 @@ export function useTreeData(options: TreeOptions): TreeData // Create a new node. If null, then delete the node, otherwise replace. let newNode = update(node); if (newNode == null) { - deleteNode(node); + deleteNode(node, map); } else { - addNode(newNode); + addNode(newNode, map); } // Walk up the tree and update each parent to refer to the new children. while (node.parentKey) { let nextParent = map.get(node.parentKey); let copy: TreeNode = { - creation: creation++, key: nextParent.key, parentKey: nextParent.parentKey, value: nextParent.value, @@ -218,17 +216,17 @@ export function useTreeData(options: TreeOptions): TreeData }; } - function addNode(node: TreeNode) { + function addNode(node: TreeNode, map) { map.set(node.key, node); for (let child of node.children) { - addNode(child); + addNode(child, map); } } - function deleteNode(node: TreeNode) { + function deleteNode(node: TreeNode, map) { map.delete(node.key); for (let child of node.children) { - deleteNode(child); + deleteNode(child, map); } } @@ -241,7 +239,7 @@ export function useTreeData(options: TreeOptions): TreeData }, insert(parentKey: Key | null, index: number, ...values: T[]) { setItems(({items, mappy: originalMap}) => { - let {items: nodes, mappy: newMap} = buildTree(values, parentKey); + let {items: nodes, mappy: newMap} = buildTree(values, parentKey, originalMap); // If parentKey is null, insert into the root. if (parentKey == null) { @@ -308,16 +306,18 @@ export function useTreeData(options: TreeOptions): TreeData remove(...keys: Key[]) { let newItems = items; let prevMap = map; + let tree; for (let key of keys) { - let tree = updateTree(newItems, key, () => null, prevMap); + tree = updateTree(newItems, key, () => null, prevMap); prevMap = tree.mappy; + newItems = tree.items; } setItems(tree); let selection = new Set(selectedKeys); for (let key of selectedKeys) { - if (!map.has(key)) { + if (!tree.mappy.has(key)) { selection.delete(key); } } @@ -373,7 +373,7 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - let tree = buildTree(getChildren(newValue), node.key); + let tree = buildTree(getChildren(newValue), node.key, originalMap); node.children = tree.items; return node; }, originalMap)); From c259466a137d9d927f464a89bea90f0b40d40e71 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 16:03:05 -0800 Subject: [PATCH 07/12] revert canary testing changes --- package.json | 4 ++-- yarn.lock | 22 ---------------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index e083518cf6b..4032e8ec4c4 100644 --- a/package.json +++ b/package.json @@ -157,9 +157,9 @@ "postcss-import": "^15.1.0", "prop-types": "^15.6.0", "raf": "^3.4.0", - "react": "^18.3.0-canary-2cd19ed1d-20240207", + "react": "^18.0.0", "react-axe": "^3.0.2", - "react-dom": "^18.3.0-canary-2cd19ed1d-20240207", + "react-dom": "^18.0.0", "react-test-renderer": "^16.9.0", "recast": "^0.20", "recursive-readdir": "^2.2.2", diff --git a/yarn.lock b/yarn.lock index be3f2454ef0..44accbaab66 100644 --- a/yarn.lock +++ b/yarn.lock @@ -21117,14 +21117,6 @@ react-dom@^18.0.0: loose-envify "^1.1.0" scheduler "^0.22.0" -react-dom@^18.3.0-canary-2cd19ed1d-20240207: - version "18.3.0-canary-2cd19ed1d-20240207" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.3.0-canary-2cd19ed1d-20240207.tgz#24759a8eb114bd8d3d543af332fb52d0872a7281" - integrity sha512-vo0KNb73J0vEGvmpr6uh+q/9579liEVU9wgVH3KbaZg4+Q7j6jcneXUvwp6Vf4xxLNNOI94CwcDcDvguLm6O6A== - dependencies: - loose-envify "^1.1.0" - scheduler "0.24.0-canary-2cd19ed1d-20240207" - react-element-to-jsx-string@^14.3.4: version "14.3.4" resolved "https://registry.yarnpkg.com/react-element-to-jsx-string/-/react-element-to-jsx-string-14.3.4.tgz#709125bc72f06800b68f9f4db485f2c7d31218a8" @@ -21213,13 +21205,6 @@ react@^18.0.0: dependencies: loose-envify "^1.1.0" -react@^18.3.0-canary-2cd19ed1d-20240207: - version "18.3.0-canary-2cd19ed1d-20240207" - resolved "https://registry.yarnpkg.com/react/-/react-18.3.0-canary-2cd19ed1d-20240207.tgz#9c1f481b71fde2d3ba3dbc38ab96e291bbb5c769" - integrity sha512-Fu00nsyCds3mH713G9/zjif2P3O8j49vNqmv1gMQjYi4SosmhOW2BlLOpOASwmHCpgPspYs8YTCDRAXkT+3o8g== - dependencies: - loose-envify "^1.1.0" - read-cache@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/read-cache/-/read-cache-1.0.0.tgz#e664ef31161166c9751cdbe8dbcf86b5fb58f774" @@ -22155,13 +22140,6 @@ saxes@^6.0.0: dependencies: xmlchars "^2.2.0" -scheduler@0.24.0-canary-2cd19ed1d-20240207: - version "0.24.0-canary-2cd19ed1d-20240207" - resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.24.0-canary-2cd19ed1d-20240207.tgz#48075b97f1afbea3ed6caf1f70ac72576f25c07f" - integrity sha512-9+0q2o4q0Nhlk7rGJf6sL08+Zp2qilbUkWCMhmDXC/codtdrzE9WqrxakqrpHet7NmZigcVyPKJFSWH6kS+uLw== - dependencies: - loose-envify "^1.1.0" - scheduler@^0.16.2: version "0.16.2" resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.16.2.tgz#f74cd9d33eff6fc554edfb79864868e4819132c1" From ec10e60e1c6fa44a14ec2183de3a8db8fa926637 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 7 Feb 2024 16:36:10 -0800 Subject: [PATCH 08/12] fix lint --- packages/@react-stately/data/src/useTreeData.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 7330a2c537e..9be40bd3c69 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -11,7 +11,7 @@ */ import {Key} from '@react-types/shared'; -import {useMemo, useState} from 'react'; +import {useState} from 'react'; export interface TreeOptions { /** Initial root items in the tree. */ From 46304e65101cff7a0308488db0a182c6c19298f8 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 8 Feb 2024 10:12:03 -0800 Subject: [PATCH 09/12] updating naming and making things a bit easier to read --- .../@react-stately/data/src/useTreeData.ts | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 9be40bd3c69..0ab440cb4fc 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -128,8 +128,8 @@ export function useTreeData(options: TreeOptions): TreeData } = options; // We only want to compute this on initial render. - let [tree, setItems] = useState<{items: TreeNode[], mappy: Map>}>(() => buildTree(initialItems)); - let {items, mappy: map} = tree; + let [tree, setItems] = useState<{items: TreeNode[], nodeMap: Map>}>(() => buildTree(initialItems)); + let {items, nodeMap} = tree; let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); @@ -151,14 +151,14 @@ export function useTreeData(options: TreeOptions): TreeData map.set(node.key, node); return node; }), - mappy: map + nodeMap: map }; } function updateTree(items: TreeNode[], key: Key, update: (node: TreeNode) => TreeNode, originalMap: Map>) { let node = originalMap.get(key); if (!node) { - return {items, mappy: originalMap}; + return {items, nodeMap: originalMap}; } let map = new Map>(originalMap); @@ -205,25 +205,24 @@ export function useTreeData(options: TreeOptions): TreeData return { items: items.map(item => { - // if (item.key === node.key) { if (item === node) { return newNode; } return item; }), - mappy: map + nodeMap: map }; } - function addNode(node: TreeNode, map) { + function addNode(node: TreeNode, map: Map>) { map.set(node.key, node); for (let child of node.children) { addNode(child, map); } } - function deleteNode(node: TreeNode, map) { + function deleteNode(node: TreeNode, map: Map>) { map.delete(node.key); for (let child of node.children) { deleteNode(child, map); @@ -235,21 +234,21 @@ export function useTreeData(options: TreeOptions): TreeData selectedKeys, setSelectedKeys, getItem(key: Key) { - return map.get(key); + return nodeMap.get(key); }, insert(parentKey: Key | null, index: number, ...values: T[]) { - setItems(({items, mappy: originalMap}) => { - let {items: nodes, mappy: newMap} = buildTree(values, parentKey, originalMap); + setItems(({items, nodeMap: originalMap}) => { + let {items: newNodes, nodeMap: newMap} = buildTree(values, parentKey, originalMap); // If parentKey is null, insert into the root. if (parentKey == null) { return { items: [ ...items.slice(0, index), - ...nodes, + ...newNodes, ...items.slice(index) ], - mappy: newMap + nodeMap: newMap }; } @@ -260,30 +259,30 @@ export function useTreeData(options: TreeOptions): TreeData value: parentNode.value, children: [ ...parentNode.children.slice(0, index), - ...nodes, + ...newNodes, ...parentNode.children.slice(index) ] }), newMap); }); }, insertBefore(key: Key, ...values: T[]): void { - let node = map.get(key); + let node = nodeMap.get(key); if (!node) { return; } - let parentNode = map.get(node.parentKey); + let parentNode = nodeMap.get(node.parentKey); let nodes = parentNode ? parentNode.children : items; let index = nodes.indexOf(node); this.insert(parentNode?.key, index, ...values); }, insertAfter(key: Key, ...values: T[]): void { - let node = map.get(key); + let node = nodeMap.get(key); if (!node) { return; } - let parentNode = map.get(node.parentKey); + let parentNode = nodeMap.get(node.parentKey); let nodes = parentNode ? parentNode.children : items; let index = nodes.indexOf(node); this.insert(parentNode?.key, index + 1, ...values); @@ -295,7 +294,7 @@ export function useTreeData(options: TreeOptions): TreeData if (parentKey == null) { this.insert(null, items.length, ...values); } else { - let parentNode = map.get(parentKey); + let parentNode = nodeMap.get(parentKey); if (!parentNode) { return; } @@ -305,19 +304,19 @@ export function useTreeData(options: TreeOptions): TreeData }, remove(...keys: Key[]) { let newItems = items; - let prevMap = map; - let tree; + let prevMap = nodeMap; + let newTree; for (let key of keys) { - tree = updateTree(newItems, key, () => null, prevMap); - prevMap = tree.mappy; - newItems = tree.items; + newTree = updateTree(newItems, key, () => null, prevMap); + prevMap = newTree.nodeMap; + newItems = newTree.items; } - setItems(tree); + setItems(newTree); let selection = new Set(selectedKeys); for (let key of selectedKeys) { - if (!tree.mappy.has(key)) { + if (!newTree.nodeMap.has(key)) { selection.delete(key); } } @@ -328,14 +327,14 @@ export function useTreeData(options: TreeOptions): TreeData this.remove(...selectedKeys); }, move(key: Key, toParentKey: Key | null, index: number) { - setItems(({items, mappy: originalMap}) => { + setItems(({items, nodeMap: originalMap}) => { let node = originalMap.get(key); if (!node) { - return {items, mappy: originalMap}; + return {items, nodeMap: originalMap}; } - let tree = updateTree(items, key, () => null, originalMap); - let {items: newItems, mappy: map} = tree; + let {items: newItems, nodeMap: newMap} = updateTree(items, key, () => null, originalMap); + const movedNode = { ...node, @@ -348,7 +347,7 @@ export function useTreeData(options: TreeOptions): TreeData ...newItems.slice(0, index), movedNode, ...newItems.slice(index) - ], mappy: map}; + ], nodeMap: newMap}; } // Otherwise, update the parent node and its ancestors. @@ -361,11 +360,11 @@ export function useTreeData(options: TreeOptions): TreeData movedNode, ...parentNode.children.slice(index) ] - }), map); + }), newMap); }); }, update(oldKey: Key, newValue: T) { - setItems(({items, mappy: originalMap}) => updateTree(items, oldKey, oldNode => { + setItems(({items, nodeMap: originalMap}) => updateTree(items, oldKey, oldNode => { let node: TreeNode = { key: oldNode.key, parentKey: oldNode.parentKey, From 173d02f31b4b4b6b1274e88b43e7268453912c16 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 8 Feb 2024 11:42:12 -0800 Subject: [PATCH 10/12] make sure a new map is always generated in insert for purity --- packages/@react-stately/data/src/useTreeData.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 0ab440cb4fc..dc424527d0f 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -238,7 +238,8 @@ export function useTreeData(options: TreeOptions): TreeData }, insert(parentKey: Key | null, index: number, ...values: T[]) { setItems(({items, nodeMap: originalMap}) => { - let {items: newNodes, nodeMap: newMap} = buildTree(values, parentKey, originalMap); + let map = new Map>(originalMap); + let {items: newNodes, nodeMap: newMap} = buildTree(values, parentKey, map); // If parentKey is null, insert into the root. if (parentKey == null) { From 8377a7cc29dc319b630dabe89d59e81fdd90e847 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 8 Feb 2024 13:22:57 -0800 Subject: [PATCH 11/12] make useTreeData always use and return a new Map this avoids the case where we may inadvertantly modify the previous map directly which could be problematic in a double render --- packages/@react-stately/data/src/useTreeData.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index dc424527d0f..891965499f3 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -128,16 +128,12 @@ export function useTreeData(options: TreeOptions): TreeData } = options; // We only want to compute this on initial render. - let [tree, setItems] = useState<{items: TreeNode[], nodeMap: Map>}>(() => buildTree(initialItems)); + let [tree, setItems] = useState<{items: TreeNode[], nodeMap: Map>}>(() => buildTree(initialItems, new Map())); let {items, nodeMap} = tree; let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); - function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc?: Map>) { - let map = acc; - if (!acc) { - map = new Map>(); - } + function buildTree(initialItems: T[] = [], map: Map>, parentKey?: Key | null) { return { items: initialItems.map(item => { let node: TreeNode = { @@ -147,7 +143,7 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - node.children = buildTree(getChildren(item), node.key, map).items; + node.children = buildTree(getChildren(item), map, node.key).items; map.set(node.key, node); return node; }), @@ -238,8 +234,7 @@ export function useTreeData(options: TreeOptions): TreeData }, insert(parentKey: Key | null, index: number, ...values: T[]) { setItems(({items, nodeMap: originalMap}) => { - let map = new Map>(originalMap); - let {items: newNodes, nodeMap: newMap} = buildTree(values, parentKey, map); + let {items: newNodes, nodeMap: newMap} = buildTree(values, originalMap, parentKey); // If parentKey is null, insert into the root. if (parentKey == null) { @@ -373,7 +368,7 @@ export function useTreeData(options: TreeOptions): TreeData children: null }; - let tree = buildTree(getChildren(newValue), node.key, originalMap); + let tree = buildTree(getChildren(newValue), originalMap, node.key); node.children = tree.items; return node; }, originalMap)); From 2103dd1f0aee53fc974464571834e5a2ed0a7981 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 8 Feb 2024 14:21:51 -0800 Subject: [PATCH 12/12] guard against empy array in remove --- packages/@react-stately/data/src/useTreeData.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/@react-stately/data/src/useTreeData.ts b/packages/@react-stately/data/src/useTreeData.ts index 891965499f3..5ad67d06e6b 100644 --- a/packages/@react-stately/data/src/useTreeData.ts +++ b/packages/@react-stately/data/src/useTreeData.ts @@ -299,6 +299,10 @@ export function useTreeData(options: TreeOptions): TreeData } }, remove(...keys: Key[]) { + if (keys.length === 0) { + return; + } + let newItems = items; let prevMap = nodeMap; let newTree;