From 9ae0511f16e1b0de482fee4ac64dca09af3be7c3 Mon Sep 17 00:00:00 2001 From: Chaiwat Ekkaewnumchai Date: Tue, 19 May 2020 09:11:12 -0700 Subject: [PATCH] Refactor Data Used And Passed to Detail Sidebar Summary: As suggested in the previous diff, I change the way to store row values. Now, (kinda) raw values are stored instead of processed values that can be used directly for `ManagedTable`. This simplifies logic in detail sidebar. I'm not sure what the effect to performance. Reviewed By: mweststrate Differential Revision: D21621896 fbshipit-source-id: 472be3caa955ca32f876f81095af21e9c17cb159 --- .../databases/DatabaseDetailSidebar.tsx | 114 +++++++----------- desktop/plugins/databases/index.tsx | 51 +++++--- 2 files changed, 74 insertions(+), 91 deletions(-) diff --git a/desktop/plugins/databases/DatabaseDetailSidebar.tsx b/desktop/plugins/databases/DatabaseDetailSidebar.tsx index 7028c3e5f..95bfe180e 100644 --- a/desktop/plugins/databases/DatabaseDetailSidebar.tsx +++ b/desktop/plugins/databases/DatabaseDetailSidebar.tsx @@ -7,8 +7,7 @@ * @format */ -import React from 'react'; -import {QueriedTable} from '.'; +import React, {useMemo} from 'react'; import { Text, DetailSidebar, @@ -17,53 +16,29 @@ import { TableRows, TableBodyRow, ManagedDataInspector, + Value, + renderValue, } from 'flipper'; -function sidebarRows(id: number, table: QueriedTable): TableRows { - const columns = table.columns; - const row = table.rows[id]; - if (columns.length === 1) { - const sidebarArray = []; - // TODO(T60896483): Narrow the scope of this try/catch block. - try { - const parsed = JSON.parse(row.columns[columns[0]].value.props.children); - for (const key in parsed) { - sidebarArray.push( - buildSidebarRow(key, { - props: { - children: parsed[key], - }, - }), - ); - } - } catch (e) { - sidebarArray.push( - buildSidebarRow(columns[0], row.columns[columns[0]].value), - ); - } - return sidebarArray; - } else { - return columns.map((column, i) => - buildSidebarRow(columns[i], row.columns[columns[i]].value), - ); - } +type DatabaseDetailSidebarProps = { + columnLabels: Array; + columnValues: Array; +}; + +function sidebarRows(labels: Array, values: Array): TableRows { + return labels.map((label, idx) => buildSidebarRow(label, values[idx])); } -function buildSidebarRow(key: string, val: any): TableBodyRow { - let output: any = ''; +function buildSidebarRow(key: string, val: Value): TableBodyRow { + let output = renderValue(val, true); // TODO(T60896483): Narrow the scope of this try/catch block. - try { - const parsed = JSON.parse(val.props.children); - for (const key in parsed) { - try { - parsed[key] = JSON.parse(parsed[key]); - } catch (err) {} - } - output = ( - - ); - } catch (error) { - output = val; + if (val.type === 'string') { + try { + const parsed = JSON.parse(val.value); + output = ( + + ); + } catch (_error) {} } return { columns: { @@ -76,32 +51,29 @@ function buildSidebarRow(key: string, val: any): TableBodyRow { }; } -export default React.memo(function DatabaseDetailSidebar(props: { - table: QueriedTable; -}) { - const {table} = props; - if ( - table.highlightedRows === null || - typeof table.highlightedRows === 'undefined' || - table.highlightedRows.length !== 1 - ) { - return null; - } - const id = table.highlightedRows[0]; - const cols = { - col: { - value: 'Column', - resizable: true, - }, - val: { - value: 'Value', - resizable: true, - }, - }; - const colSizes = { - col: '35%', - val: 'flex', - }; +const cols = { + col: { + value: 'Column', + resizable: true, + }, + val: { + value: 'Value', + resizable: true, + }, +}; +const colSizes = { + col: '35%', + val: 'flex', +}; + +export default React.memo(function DatabaseDetailSidebar( + props: DatabaseDetailSidebarProps, +) { + const {columnLabels, columnValues} = props; + const rows = useMemo(() => sidebarRows(columnLabels, columnValues), [ + columnLabels, + columnValues, + ]); return ( diff --git a/desktop/plugins/databases/index.tsx b/desktop/plugins/databases/index.tsx index a9481327f..c01c319bd 100644 --- a/desktop/plugins/databases/index.tsx +++ b/desktop/plugins/databases/index.tsx @@ -26,13 +26,14 @@ import { TableBodyColumn, TableRows, Props as FlipperPluginProps, + TableBodyRow, + TableRowSortOrder, + FlipperPlugin, + Value, + renderValue, } from 'flipper'; import React, {Component, KeyboardEvent, ChangeEvent} from 'react'; -import {TableBodyRow, TableRowSortOrder} from 'flipper'; -import {FlipperPlugin} from 'flipper'; import {DatabaseClient} from './ClientProtocol'; -import {renderValue} from 'flipper'; -import {Value} from 'flipper'; import ButtonNavigation from './ButtonNavigation'; import DatabaseDetailSidebar from './DatabaseDetailSidebar'; import sqlFormatter from 'sql-formatter'; @@ -86,7 +87,7 @@ type Page = { databaseId: number; table: string; columns: Array; - rows: Array; + rows: Array>; start: number; count: number; total: number; @@ -110,7 +111,7 @@ type QueryResult = { export type QueriedTable = { columns: Array; - rows: Array; + rows: Array>; highlightedRows: Array; }; @@ -170,7 +171,7 @@ type UpdatePageEvent = { databaseId: number; table: string; columns: Array; - values: Array>; + values: Array>; start: number; count: number; total: number; @@ -181,15 +182,15 @@ type UpdateStructureEvent = { databaseId: number; table: string; columns: Array; - rows: Array>; + rows: Array>; indexesColumns: Array; - indexesValues: Array>; + indexesValues: Array>; }; type DisplaySelectEvent = { type: 'DisplaySelect'; columns: Array; - values: Array>; + values: Array>; }; type DisplayInsertEvent = { @@ -529,9 +530,7 @@ export default class DatabasesPlugin extends FlipperPlugin< return { ...state, currentPage: { - rows: event.values.map((row: Array, index: number) => - transformRow(event.columns, row, index), - ), + rows: event.values, highlightedRows: [], ...event, }, @@ -573,9 +572,7 @@ export default class DatabasesPlugin extends FlipperPlugin< queryResult: { table: { columns: event.columns, - rows: event.values.map((row: Array, index: number) => - transformRow(event.columns, row, index), - ), + rows: event.values, highlightedRows: [], }, id: null, @@ -1083,7 +1080,9 @@ export default class DatabasesPlugin extends FlipperPlugin< {}, )} zebra={true} - rows={page.rows} + rows={page.rows.map((row: Array, index: number) => + transformRow(page.columns, row, index), + )} horizontallyScrollable={true} multiHighlight={true} onRowHighlighted={(highlightedRows) => @@ -1105,7 +1104,12 @@ export default class DatabasesPlugin extends FlipperPlugin< }} initialSortOrder={this.state.currentSort ?? undefined} /> - + {page.highlightedRows.length === 1 && ( + + )} ); } @@ -1137,7 +1141,9 @@ export default class DatabasesPlugin extends FlipperPlugin< {}, )} zebra={true} - rows={rows} + rows={rows.map((row: Array, index: number) => + transformRow(columns, row, index), + )} horizontallyScrollable={true} onRowHighlighted={(highlightedRows) => { this.setState({ @@ -1153,7 +1159,12 @@ export default class DatabasesPlugin extends FlipperPlugin< }); }} /> - + {table.highlightedRows.length === 1 && ( + + )} ); } else if (query.id && query.id !== null) {