-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Pure render for tree data, fixes React Canary useMemo/useState StrictMode #5835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
c9e7d13
Pure render for useTreeData
snowystinger ea85e9e
adjust some more calls
snowystinger 0257054
fix accumulation of build trees
snowystinger 86d3a0f
better readability
snowystinger 62a7fef
fix typo
snowystinger ca1545a
fix remaining broken tests
snowystinger c259466
revert canary testing changes
snowystinger ec10e60
fix lint
snowystinger 46304e6
updating naming and making things a bit easier to read
LFDanLu 173d02f
make sure a new map is always generated in insert for purity
LFDanLu 8377a7c
make useTreeData always use and return a new Map
LFDanLu 2103dd1
guard against empy array in remove
LFDanLu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
better readability
- Loading branch information
commit 86d3a0f9c920fde5ad1683205a2d7142443aadf1
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,35 +128,38 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): 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<T>[], mappy: Map<Key, TreeNode<T>>}>(() => buildTree(initialItems)); | ||
| let {items, mappy: map} = tree; | ||
|
|
||
| let [selectedKeys, setSelectedKeys] = useState(new Set<Key>(initialSelectedKeys || [])); | ||
|
|
||
| function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc: Map<Key, TreeNode<T>>) { | ||
| function buildTree(initialItems: T[] = [], parentKey?: Key | null, acc?: Map<Key, TreeNode<T>>) { | ||
| let map = acc; | ||
| if (!acc) { | ||
| map = new Map<Key, TreeNode<T>>(); | ||
| } | ||
| return [initialItems.map(item => { | ||
| let node: TreeNode<T> = { | ||
| creation: creation++, | ||
| key: getKey(item), | ||
| parentKey: parentKey, | ||
| value: item, | ||
| children: null | ||
| }; | ||
| return { | ||
| items: initialItems.map(item => { | ||
| let node: TreeNode<T> = { | ||
| 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<T>[], key: Key, update: (node: TreeNode<T>) => TreeNode<T>, originalMap: Map<Key, TreeNode<T>>) { | ||
| let node = originalMap.get(key); | ||
| if (!node) { | ||
| return [items, originalMap]; | ||
| return {items, mappy: originalMap}; | ||
|
||
| } | ||
| let map = new Map<Key, TreeNode<T>>(originalMap); | ||
|
|
||
|
|
@@ -202,14 +205,17 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): 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) { | ||
LFDanLu marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (item === node) { | ||
| return newNode; | ||
| } | ||
|
|
||
| return item; | ||
| }), map]; | ||
| return item; | ||
| }), | ||
| mappy: map | ||
| }; | ||
| } | ||
|
|
||
| function addNode(node: TreeNode<T>) { | ||
|
|
@@ -234,16 +240,19 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): 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<T extends object>(options: TreeOptions<T>): TreeData | |
| ...nodes, | ||
| ...parentNode.children.slice(index) | ||
| ] | ||
| }), originalMap); | ||
| }), newMap); | ||
| }); | ||
| }, | ||
| insertBefore(key: Key, ...values: T[]): void { | ||
|
|
@@ -300,11 +309,11 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): 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<T extends object>(options: TreeOptions<T>): 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<T extends object>(options: TreeOptions<T>): 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<T extends object>(options: TreeOptions<T>): 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<T> = { | ||
| key: oldNode.key, | ||
| parentKey: oldNode.parentKey, | ||
|
|
@@ -364,8 +374,8 @@ export function useTreeData<T extends object>(options: TreeOptions<T>): TreeData | |
| }; | ||
|
|
||
| let tree = buildTree(getChildren(newValue), node.key); | ||
| node.children = tree[0]; | ||
| return [node, tree[1]]; | ||
| node.children = tree.items; | ||
| return node; | ||
| }, originalMap)); | ||
| } | ||
| }; | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it called
mappy? 😂