From 7d9495027b68ec466b542929dfac8ee367fd7106 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 15 Apr 2021 07:46:28 -0700 Subject: [PATCH] Fix layout inspector re-rendering all elements on all changes Summary: Changelog: [Layout] Addressed several performance issues in the layout plugin This diff and a few of the next stuff fix some performance issues in the Layout plugin. This diff fixes an issue where computing the context menu will cause all rows to render at all times, make the responiveness of the plugin quite slugish. The fix in this case is to build up the context menu lazily, and pass a stable ref to the function through the tree, rather than a new menu every time the root component renders. The changes in this diff and the next ones in total reduces the time (in prod builds) to draw a frame from ~200ms to ~5ms. Reviewed By: cekkaewnumchai Differential Revision: D27685983 fbshipit-source-id: a48b2ce2cdd1db31bb13122924617cbc3b6c198a --- .../ui/elements-inspector/ElementsInspector.tsx | 2 +- .../src/ui/elements-inspector/elements.tsx | 16 +++++++++++----- desktop/plugins/public/layout/Inspector.tsx | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/desktop/flipper-plugin/src/ui/elements-inspector/ElementsInspector.tsx b/desktop/flipper-plugin/src/ui/elements-inspector/ElementsInspector.tsx index 591751477..78c6e0b9e 100644 --- a/desktop/flipper-plugin/src/ui/elements-inspector/ElementsInspector.tsx +++ b/desktop/flipper-plugin/src/ui/elements-inspector/ElementsInspector.tsx @@ -70,7 +70,7 @@ export type ElementsInspectorProps = { elements: {[key: string]: Element}; useAppSidebar?: boolean; alternateRowColor?: boolean; - contextMenuExtensions?: Array; + contextMenuExtensions?: () => Array; decorateRow?: DecorateRow; }; diff --git a/desktop/flipper-plugin/src/ui/elements-inspector/elements.tsx b/desktop/flipper-plugin/src/ui/elements-inspector/elements.tsx index 21fcf20a1..1b705621d 100644 --- a/desktop/flipper-plugin/src/ui/elements-inspector/elements.tsx +++ b/desktop/flipper-plugin/src/ui/elements-inspector/elements.tsx @@ -227,7 +227,7 @@ type ElementsRowProps = { | null; onCopyExpandedTree: (key: Element, maxDepth: number) => string; style?: Object; - contextMenuExtensions: Array; + contextMenuExtensions?: () => Array; decorateRow?: DecorateRow; forwardedRef: React.Ref | null; }; @@ -286,12 +286,18 @@ class ElementsRow extends PureComponent { }), ); - for (const extension of props.contextMenuExtensions) { + // Array.isArray check for backward compatibility + const extensions: ContextMenuExtension[] | undefined = Array.isArray( + props.contextMenuExtensions, + ) + ? props.contextMenuExtensions + : props.contextMenuExtensions?.(); + extensions?.forEach((extension) => { items.push({ label: extension.label, click: () => extension.click(this.props.id), }); - } + }); return ( @@ -460,7 +466,7 @@ type ElementsProps = { | undefined | null; alternateRowColor?: boolean; - contextMenuExtensions?: Array; + contextMenuExtensions?: () => Array; decorateRow?: DecorateRow; }; @@ -728,7 +734,7 @@ export class Elements extends PureComponent { isQueryMatch={containsKeyInSearchResults(searchResults, row.key)} element={row.element} childrenCount={childrenCount} - contextMenuExtensions={contextMenuExtensions || []} + contextMenuExtensions={contextMenuExtensions} decorateRow={decorateRow} forwardedRef={ selected == row.key && this.state.scrolledElement !== selected diff --git a/desktop/plugins/public/layout/Inspector.tsx b/desktop/plugins/public/layout/Inspector.tsx index f7a3a5c17..b47219308 100644 --- a/desktop/plugins/public/layout/Inspector.tsx +++ b/desktop/plugins/public/layout/Inspector.tsx @@ -458,7 +458,7 @@ export default class Inspector extends Component { root={this.root()} elements={this.elements()} focused={this.focused()} - contextMenuExtensions={this.getAXContextMenuExtensions()} + contextMenuExtensions={this.getAXContextMenuExtensions} /> {selectorData && selectorData.leaves.length > 1 ? (