From e9d098b9cdf37425f2afdfbf67bbe81a6c628af0 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Mon, 19 Jun 2023 05:06:52 -0700 Subject: [PATCH] Virtualise Tree Summary: Removed previous memoization approach as it was causing issues and very inconsistent perf increase due to the indent guides often causing half the tree needed to rerender. New approach is using react virtual. Its very fast in all cases including initial render off the wire. It does require 2 hacks. 1) React virtual requires you to explicitly size your parent component. In all the examples they have a height in px. This doesnt really work given we can resize the window. To mitigate this I added a grandparent component that is sized correclty with flexbox, then i use a layout effect to grab the height from the grandparent ref and set it to the scroll parent ref 2) Due to the implementaion of react virtual the width of the items in the tree is not correct. By default all the content overflows the box and the box doesnt grow automatically to fill the content. I think this is due to absolute positioning which breaks all the normal layout engine rules. The fix is to get the scrollWidth of the parent scroll view. (scrollWidth is the width of the element including overflow) and then set it via the refs we have on the tree item elements. This is also done in a layout effect. changelog: UIDebugger virtualized UI to improve rendering performance Reviewed By: aigoncharov Differential Revision: D46724776 fbshipit-source-id: 75a6d35542066bd788aa4536481dedc72f667fc1 --- .../public/ui-debugger/components/Tree.tsx | 183 +++++++++++------- .../plugins/public/ui-debugger/package.json | 1 + desktop/plugins/public/yarn.lock | 12 ++ 3 files changed, 130 insertions(+), 66 deletions(-) diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx index e86274c7d..842ed9122 100644 --- a/desktop/plugins/public/ui-debugger/components/Tree.tsx +++ b/desktop/plugins/public/ui-debugger/components/Tree.tsx @@ -27,16 +27,17 @@ import { useHighlighter, usePlugin, useValue, - Layout, } from 'flipper-plugin'; import {plugin} from '../index'; import {Glyph} from 'flipper'; -import {head, isEqual, last} from 'lodash'; +import {head, last} from 'lodash'; import {reverse} from 'lodash/fp'; import {Badge, Dropdown, Menu, Typography} from 'antd'; import {UIDebuggerMenuItem} from './util/UIDebuggerMenuItem'; import {tracker} from '../tracker'; +import {useVirtualizer} from '@tanstack/react-virtual'; + const {Text} = Typography; type LineStyle = 'ToParent' | 'ToChildren'; @@ -85,6 +86,16 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { const isUsingKBToScroll = useRef(false); + const grandParentRef = useRef(null); + const parentRef = React.useRef(null); + + const rowVirtualizer = useVirtualizer({ + count: treeNodes.length, + getScrollElement: () => parentRef.current, + estimateSize: () => 26, + overscan: 20, + }); + useKeyboardShortcuts( treeNodes, refs, @@ -96,18 +107,47 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { isUsingKBToScroll, ); - const scrollContainerRef = useRef(null); + useLayoutEffect(() => { + //the grand parent gets its size correclty via flex box, we use its initial + //position to size the scroll parent ref for react virtual, It uses vh which accounts for window size changes + //However if we dynamically add content above or below we may need to revisit this approach + const boundingClientRect = grandParentRef?.current?.getBoundingClientRect(); + + parentRef.current!!.style.height = `calc(100vh - ${ + boundingClientRect!!.top + }px - ${window.innerHeight - boundingClientRect!!.bottom}px )`; + }, []); + + useLayoutEffect(() => { + //scroll width is the width of the element including overflow, we grab the scroll width + //of the parent scroll container and set each divs actual width to this to make sure the + //size is correct for the selection and hover states + + const range = rowVirtualizer.range; + const end = Math.min( + refs.length, + range.endIndex + 1, //need to add 1 extra otherwise last one doesnt get the treatment + ); + + const width = parentRef.current?.scrollWidth ?? 0; + + for (let i = range.startIndex; i < end; i++) { + //set the width explicitly of all tree items to parent scroll width + const ref = refs[i]; + if (ref.current) { + ref.current.style.width = `${width}px`; + } + } + }); useLayoutEffect(() => { if (selectedNode) { const idx = treeNodes.findIndex((node) => node.id === selectedNode); if (idx !== -1) { - scrollContainerRef.current!!.scrollLeft = + parentRef.current!!.scrollLeft = Math.max(0, treeNodes[idx].depth - 10) * renderDepthOffset; - refs[idx].current?.scrollIntoView({ - block: 'nearest', - }); + rowVirtualizer.scrollToIndex(idx, {align: 'auto'}); } } // NOTE: We don't want to add refs or tree nodes to the dependency list since when new data comes in over the wire @@ -115,69 +155,70 @@ export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { // We only should scroll when selection changes // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedNode, focusedNode]); + return ( - - - + + +
{ - if (isContextMenuOpen === false) { - instance.uiState.hoveredNodes.set([]); - } + //this is scrollable div is expected by react virtual, see their docs + ref={parentRef} + style={{ + height: 0, //this get replaced by an effect + overflow: 'auto', }}> - {treeNodes.map((treeNode, index) => ( - - ))} +
{ + if (isContextMenuOpen === false) { + instance.uiState.hoveredNodes.set([]); + } + }}> + {rowVirtualizer.getVirtualItems().map((virtualRow) => ( + + ))} +
- - - +
+ + ); } -const MemoTreeItemContainer = React.memo( - TreeItemContainer, - (prevProps, nextProps) => { - const id = nextProps.treeNode.id; - return ( - prevProps.treeNode.id === nextProps.treeNode.id && - prevProps.treeNode.isExpanded === nextProps.treeNode.isExpanded && - prevProps.isContextMenuOpen === nextProps.isContextMenuOpen && - prevProps.frameworkEvents === nextProps.frameworkEvents && - prevProps.highlightedNodes === nextProps.highlightedNodes && - prevProps.frameworkEventsMonitoring === - nextProps.frameworkEventsMonitoring && - prevProps.hoveredNode !== id && //make sure that prev or next hover/selected node doesnt concern this tree node - nextProps.hoveredNode !== id && - prevProps.selectedNode !== id && - nextProps.selectedNode !== id && - isEqual(prevProps.treeNode.indentGuide, nextProps.treeNode.indentGuide) - ); - }, -); - function IndentGuide({indentGuide}: {indentGuide: NodeIndentGuide}) { const verticalLinePadding = `${renderDepthOffset * indentGuide.depth + 8}px`; @@ -205,6 +246,7 @@ function IndentGuide({indentGuide}: {indentGuide: NodeIndentGuide}) { } function TreeItemContainer({ + transform, innerRef, treeNode, frameworkEvents, @@ -219,6 +261,7 @@ function TreeItemContainer({ onCollapseNode, onHoverNode, }: { + transform: string; innerRef: Ref; treeNode: TreeNode; frameworkEvents: Map; @@ -239,12 +282,20 @@ function TreeItemContainer({ } return ( -
+
{treeNode.indentGuide != null && ( )} )} trigger={['contextMenu']}> -
{children}
+ {children} ); }; diff --git a/desktop/plugins/public/ui-debugger/package.json b/desktop/plugins/public/ui-debugger/package.json index 710946226..f5f44f452 100644 --- a/desktop/plugins/public/ui-debugger/package.json +++ b/desktop/plugins/public/ui-debugger/package.json @@ -17,6 +17,7 @@ "react-color": "^2.19.3", "react-hotkeys-hook": "^3.4.7", "react-query": "^3.39.1", + "@tanstack/react-virtual": "3.0.0-beta.54", "ts-retry-promise": "^0.7.0", "memoize-weak": "^1.0.2" }, diff --git a/desktop/plugins/public/yarn.lock b/desktop/plugins/public/yarn.lock index b478d0e84..a359e722c 100644 --- a/desktop/plugins/public/yarn.lock +++ b/desktop/plugins/public/yarn.lock @@ -155,6 +155,18 @@ estree-walker "^1.0.1" picomatch "^2.2.2" +"@tanstack/react-virtual@3.0.0-beta.54": + version "3.0.0-beta.54" + resolved "https://registry.yarnpkg.com/@tanstack/react-virtual/-/react-virtual-3.0.0-beta.54.tgz#755979455adf13f2584937204a3f38703e446037" + integrity sha512-D1mDMf4UPbrtHRZZriCly5bXTBMhylslm4dhcHqTtDJ6brQcgGmk8YD9JdWBGWfGSWPKoh2x1H3e7eh+hgPXtQ== + dependencies: + "@tanstack/virtual-core" "3.0.0-beta.54" + +"@tanstack/virtual-core@3.0.0-beta.54": + version "3.0.0-beta.54" + resolved "https://registry.yarnpkg.com/@tanstack/virtual-core/-/virtual-core-3.0.0-beta.54.tgz#12259d007911ad9fce1388385c54a9141f4ecdc4" + integrity sha512-jtkwqdP2rY2iCCDVAFuaNBH3fiEi29aTn2RhtIoky8DTTiCdc48plpHHreLwmv1PICJ4AJUUESaq3xa8fZH8+g== + "@testing-library/dom@^7.28.1": version "7.29.4" resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-7.29.4.tgz#1647c2b478789621ead7a50614ad81ab5ae5b86c"