From 1af119599756e68ee9260df7f17e83196326c745 Mon Sep 17 00:00:00 2001 From: Fabian Kovacs Date: Thu, 21 Mar 2024 13:07:00 +0100 Subject: [PATCH 1/3] Simplify Table rendering. Move allocations out of Cell-functions, avoiding unnecessary allocations --- frontend/src/js/api/api.ts | 2 +- frontend/src/js/preview/Table.tsx | 73 +++++------------------- frontend/src/js/preview/actions.ts | 2 +- frontend/src/js/preview/tableUtils.ts | 80 +++++++++++++++------------ frontend/src/js/preview/util.ts | 32 +++++------ 5 files changed, 76 insertions(+), 113 deletions(-) diff --git a/frontend/src/js/api/api.ts b/frontend/src/js/api/api.ts index 04138e7c67..a5e779c2fb 100644 --- a/frontend/src/js/api/api.ts +++ b/frontend/src/js/api/api.ts @@ -418,7 +418,7 @@ export const useGetResult = () => { [authToken], ); return useCallback( - (queryId: string, limit = 1000) => { + (queryId: string, limit: number) => { const url = `/result/arrow/${queryId}.arrs?` + new URLSearchParams({ limit: limit.toString() }); diff --git a/frontend/src/js/preview/Table.tsx b/frontend/src/js/preview/Table.tsx index 0355c64523..8ae91ac80d 100644 --- a/frontend/src/js/preview/Table.tsx +++ b/frontend/src/js/preview/Table.tsx @@ -6,8 +6,7 @@ import { Vector, } from "apache-arrow"; import RcTable from "rc-table"; -import { DefaultRecordType } from "rc-table/lib/interface"; -import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { memo, useMemo, useRef } from "react"; import { GetQueryResponseDoneT, GetQueryResponseT } from "../api/types"; import { useCustomTableRenderers } from "./tableUtils"; @@ -63,79 +62,35 @@ export default memo(function Table({ queryData, }: Props) { const rootRef = useRef(null); - const { getRenderFunctionByFieldName } = useCustomTableRenderers( - queryData as GetQueryResponseDoneT, - ); + const { getRenderFunctionByFieldName } = useCustomTableRenderers(queryData as GetQueryResponseDoneT); const columns = useMemo( () => - arrowReader.schema?.fields.map((field) => ({ + arrowReader.schema?.fields.map((field) => { + const renderer = getRenderFunctionByFieldName(field.name) + + return ({ title: field.name.charAt(0).toUpperCase() + field.name.slice(1), dataIndex: field.name, key: field.name, render: (value: string | Vector) => { - return typeof value === "string" ? ( - {value} - ) : ( - value - ); + const rendered = renderer(value) + return {rendered}; }, - })), - [arrowReader.schema], - ); - - const parseTableRows = useCallback( - (data: Vector[]) => { - const nextRows = [] as DefaultRecordType[]; - data.forEach((dataEntry: Vector) => { - const parsedValues = Object.fromEntries( - Object.entries(dataEntry.toJSON()).map(([key, value]) => { - const parsedValue = - getRenderFunctionByFieldName(key)?.(value) ?? value; - return [key, parsedValue]; - }), - ); - nextRows.push(parsedValues); - }); - - return nextRows; - }, - [getRenderFunctionByFieldName], + })}), + [arrowReader.schema, getRenderFunctionByFieldName, queryData], ); - + const loadedTableData = useMemo( - () => parseTableRows(new ArrowTable(initialTableData.value).toArray()), - [initialTableData, parseTableRows], + () => new ArrowTable(initialTableData.value).toArray(), + [initialTableData], ); - const [visibleTableRows, setVisibleTableRows] = useState(50); - - useEffect(() => { - const eventFunction = async () => { - const div = rootRef.current; - if (!div) { - return; - } - const maxScroll = - (div.parentElement?.scrollHeight || div.scrollHeight) - - window.innerHeight; - const thresholdTriggered = - (div.parentElement?.scrollTop || div.scrollTop) / maxScroll > 0.9; - if (thresholdTriggered) { - setVisibleTableRows((rowCount) => - Math.min(rowCount + 50, loadedTableData.length), - ); - } - }; - - window.addEventListener("scroll", eventFunction, true); - return () => window.removeEventListener("scroll", eventFunction, true); - }, [loadedTableData, visibleTableRows, arrowReader]); return ( `previewtable_row_${index}`} components={{ table: StyledTable, diff --git a/frontend/src/js/preview/actions.ts b/frontend/src/js/preview/actions.ts index cf856360c7..905013e0dc 100644 --- a/frontend/src/js/preview/actions.ts +++ b/frontend/src/js/preview/actions.ts @@ -81,7 +81,7 @@ export function useLoadPreviewData() { try { const arrowReader = await AsyncRecordBatchStreamReader.from( - getResult(queryId), + getResult(queryId, 100), ); const loadInitialData = async () => { await arrowReader.open(); diff --git a/frontend/src/js/preview/tableUtils.ts b/frontend/src/js/preview/tableUtils.ts index ea727e13e9..dfff05b6b7 100644 --- a/frontend/src/js/preview/tableUtils.ts +++ b/frontend/src/js/preview/tableUtils.ts @@ -5,10 +5,7 @@ import { useSelector } from "react-redux"; import { CurrencyConfigT, GetQueryResponseDoneT } from "../api/types"; import { StateT } from "../app/reducers"; import { - NUMBER_TYPES, - formatDate, - formatNumber, - toFullLocaleDateString, + NUMBER_TYPES, currencyFromSymbol } from "./util"; export type CellValue = string | Vector; @@ -19,8 +16,16 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) { (state) => state.startup.config.currency, ); + const dateFormatter = new Intl.DateTimeFormat(navigator.language, { + day: "2-digit", + month: "2-digit", + year: "numeric", + }) + + const currencyFormatter = new Intl.NumberFormat(navigator.language, { style: "currency", currency: currencyFromSymbol(currencyConfig.unit) }) + const getRenderFunction = useCallback( - (cellType: string): ((value: CellValue) => string) | undefined => { + (cellType: string): ((value: CellValue) => string) => { if (cellType.indexOf("LIST") == 0) { const listType = cellType.match(/LIST\[(?.*)\]/)?.groups?.[ "listtype" @@ -30,59 +35,64 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) { return (value) => value ? (value as Vector) - .toArray() - .map((listItem: string) => - listTypeRenderFunction - ? listTypeRenderFunction(listItem) - : listItem, - ) + .toArray() // This is somewhat slow, but for-loop produces bogus values + .map(listTypeRenderFunction) .join(", ") : null; } } else if (NUMBER_TYPES.includes(cellType)) { + const numnberFormatter = new Intl.NumberFormat(navigator.language, { + maximumFractionDigits: 2, + minimumFractionDigits: cellType == "INTEGER" ? 0 : 2, + }) + return (value) => { - const num = parseFloat(value as string); - return isNaN(num) ? "" : formatNumber(num); + if (value && !isNaN(value as unknown as number)) { + return numnberFormatter.format(value as unknown as number) + } + return ""; }; } else if (cellType == "DATE") { - return (value) => - value instanceof Date - ? toFullLocaleDateString(value) - : formatDate(value as string); + return (value) => dateFormatter.format(value as unknown as Date); } else if (cellType == "DATE_RANGE") { + return (value) => { - const dateRange = (value as Vector).toJSON() as unknown as { - min: Date; - max: Date; - }; - const min = toFullLocaleDateString(dateRange.min); - const max = toFullLocaleDateString(dateRange.max); - return min == max ? min : `${min} - ${max}`; + const vector = value as unknown as { min: Date, max: Date }; + + const min = dateFormatter.format(vector.min); + const max = dateFormatter.format(vector.max); + + if (min == max) { + return min + } + + return `${min} - ${max}`; }; - } else if (cellType == "MONEY") { + } else if (cellType == "MONEY") { return (value) => { - const num = parseFloat(value as string) / 100; - return isNaN(num) - ? "" - : `${formatNumber(num, { forceFractionDigits: true })} ${ - currencyConfig.unit - }`; + if (value && !isNaN(value as unknown as number)) { + return currencyFormatter.format(value as unknown as number) + } + return "" }; } else if (cellType == "BOOLEAN") { return (value) => (value ? t("common.true") : t("common.false")); } + + return (value) => value ? value as string : "" }, [currencyConfig.unit, t], ); const getRenderFunctionByFieldName = useCallback( - (fieldName: string): ((value: CellValue) => string) | undefined => { - const cellType = ( - queryData as GetQueryResponseDoneT - ).columnDescriptions?.find((x) => x.label == fieldName)?.type; + (fieldName: string): ((value: CellValue) => string) => { + const cellType = (queryData as GetQueryResponseDoneT).columnDescriptions?.find((x) => x.label == fieldName)?.type; + if (cellType) { return getRenderFunction(cellType); } + + return (value) => value ? value as string : "" }, [getRenderFunction, queryData], ); diff --git a/frontend/src/js/preview/util.ts b/frontend/src/js/preview/util.ts index dad51022b4..7e4695c184 100644 --- a/frontend/src/js/preview/util.ts +++ b/frontend/src/js/preview/util.ts @@ -1,11 +1,24 @@ -import { t } from "i18next"; import { BarStatistics, DateStatistics, PreviewStatistics } from "../api/types"; -import { parseDate } from "../common/helpers/dateHelper"; export const NUMBER_TYPES = ["NUMERIC", "INTEGER"]; export const NUMBER_STATISTICS_TYPES = [...NUMBER_TYPES, "MONEY"]; + + +export function currencyFromSymbol(symbol: string) : string { + //TODO this is a workaround until the backend sends currency-codes + if(symbol == "€"){ + return "EUR" + } + + if(symbol == "$"){ + return "USD" + } + + return "USD" +} + export function formatNumber( num: number, { @@ -19,21 +32,6 @@ export function formatNumber( }).format(num); } -export function formatDate(date: string | undefined) { - if (date) { - const parsedDate = parseDate(date, "yyyy-MM-dd"); - return parsedDate ? toFullLocaleDateString(parsedDate) : date; - } - return t("preview.dateError"); -} - -export function toFullLocaleDateString(date: Date) { - return date.toLocaleDateString(navigator.language, { - day: "2-digit", - month: "2-digit", - year: "numeric", - }); -} export function previewStatsIsBarStats( stats: PreviewStatistics, From b16715d925c3b4cfe166d0c90f56255e98064811 Mon Sep 17 00:00:00 2001 From: Fabian Kovacs Date: Thu, 21 Mar 2024 14:06:03 +0100 Subject: [PATCH 2/3] lint and format --- frontend/src/js/preview/Table.tsx | 31 ++++++++------- frontend/src/js/preview/tableUtils.ts | 56 ++++++++++++++------------- frontend/src/js/preview/util.ts | 15 +++---- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/frontend/src/js/preview/Table.tsx b/frontend/src/js/preview/Table.tsx index 8ae91ac80d..c019a38995 100644 --- a/frontend/src/js/preview/Table.tsx +++ b/frontend/src/js/preview/Table.tsx @@ -62,25 +62,28 @@ export default memo(function Table({ queryData, }: Props) { const rootRef = useRef(null); - const { getRenderFunctionByFieldName } = useCustomTableRenderers(queryData as GetQueryResponseDoneT); + const { getRenderFunctionByFieldName } = useCustomTableRenderers( + queryData as GetQueryResponseDoneT, + ); const columns = useMemo( () => arrowReader.schema?.fields.map((field) => { - const renderer = getRenderFunctionByFieldName(field.name) - - return ({ - title: field.name.charAt(0).toUpperCase() + field.name.slice(1), - dataIndex: field.name, - key: field.name, - render: (value: string | Vector) => { - const rendered = renderer(value) - return {rendered}; - }, - })}), - [arrowReader.schema, getRenderFunctionByFieldName, queryData], + const renderer = getRenderFunctionByFieldName(field.name); + + return { + title: field.name.charAt(0).toUpperCase() + field.name.slice(1), + dataIndex: field.name, + key: field.name, + render: (value: string | Vector) => { + const rendered = renderer(value); + return {rendered}; + }, + }; + }), + [arrowReader.schema, getRenderFunctionByFieldName], ); - + const loadedTableData = useMemo( () => new ArrowTable(initialTableData.value).toArray(), [initialTableData], diff --git a/frontend/src/js/preview/tableUtils.ts b/frontend/src/js/preview/tableUtils.ts index dfff05b6b7..db3cd14db8 100644 --- a/frontend/src/js/preview/tableUtils.ts +++ b/frontend/src/js/preview/tableUtils.ts @@ -4,9 +4,7 @@ import { useTranslation } from "react-i18next"; import { useSelector } from "react-redux"; import { CurrencyConfigT, GetQueryResponseDoneT } from "../api/types"; import { StateT } from "../app/reducers"; -import { - NUMBER_TYPES, currencyFromSymbol -} from "./util"; +import { NUMBER_TYPES, currencyFromSymbol } from "./util"; export type CellValue = string | Vector; @@ -16,16 +14,19 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) { (state) => state.startup.config.currency, ); - const dateFormatter = new Intl.DateTimeFormat(navigator.language, { - day: "2-digit", - month: "2-digit", - year: "numeric", - }) - - const currencyFormatter = new Intl.NumberFormat(navigator.language, { style: "currency", currency: currencyFromSymbol(currencyConfig.unit) }) - const getRenderFunction = useCallback( (cellType: string): ((value: CellValue) => string) => { + const dateFormatter = new Intl.DateTimeFormat(navigator.language, { + day: "2-digit", + month: "2-digit", + year: "numeric", + }); + + const currencyFormatter = new Intl.NumberFormat(navigator.language, { + style: "currency", + currency: currencyFromSymbol(currencyConfig.unit), + }); + if (cellType.indexOf("LIST") == 0) { const listType = cellType.match(/LIST\[(?.*)\]/)?.groups?.[ "listtype" @@ -44,55 +45,56 @@ export function useCustomTableRenderers(queryData: GetQueryResponseDoneT) { const numnberFormatter = new Intl.NumberFormat(navigator.language, { maximumFractionDigits: 2, minimumFractionDigits: cellType == "INTEGER" ? 0 : 2, - }) - + }); + return (value) => { if (value && !isNaN(value as unknown as number)) { - return numnberFormatter.format(value as unknown as number) + return numnberFormatter.format(value as unknown as number); } return ""; }; } else if (cellType == "DATE") { return (value) => dateFormatter.format(value as unknown as Date); } else if (cellType == "DATE_RANGE") { - return (value) => { - const vector = value as unknown as { min: Date, max: Date }; - + const vector = value as unknown as { min: Date; max: Date }; + const min = dateFormatter.format(vector.min); const max = dateFormatter.format(vector.max); - + if (min == max) { - return min + return min; } - + return `${min} - ${max}`; }; - } else if (cellType == "MONEY") { + } else if (cellType == "MONEY") { return (value) => { if (value && !isNaN(value as unknown as number)) { - return currencyFormatter.format(value as unknown as number) + return currencyFormatter.format(value as unknown as number); } - return "" + return ""; }; } else if (cellType == "BOOLEAN") { return (value) => (value ? t("common.true") : t("common.false")); } - return (value) => value ? value as string : "" + return (value) => (value ? (value as string) : ""); }, [currencyConfig.unit, t], ); const getRenderFunctionByFieldName = useCallback( (fieldName: string): ((value: CellValue) => string) => { - const cellType = (queryData as GetQueryResponseDoneT).columnDescriptions?.find((x) => x.label == fieldName)?.type; - + const cellType = ( + queryData as GetQueryResponseDoneT + ).columnDescriptions?.find((x) => x.label == fieldName)?.type; + if (cellType) { return getRenderFunction(cellType); } - return (value) => value ? value as string : "" + return (value) => (value ? (value as string) : ""); }, [getRenderFunction, queryData], ); diff --git a/frontend/src/js/preview/util.ts b/frontend/src/js/preview/util.ts index 7e4695c184..0b5814a646 100644 --- a/frontend/src/js/preview/util.ts +++ b/frontend/src/js/preview/util.ts @@ -4,19 +4,17 @@ export const NUMBER_TYPES = ["NUMERIC", "INTEGER"]; export const NUMBER_STATISTICS_TYPES = [...NUMBER_TYPES, "MONEY"]; - - -export function currencyFromSymbol(symbol: string) : string { +export function currencyFromSymbol(symbol: string): string { //TODO this is a workaround until the backend sends currency-codes - if(symbol == "€"){ - return "EUR" + if (symbol == "€") { + return "EUR"; } - if(symbol == "$"){ - return "USD" + if (symbol == "$") { + return "USD"; } - return "USD" + return "USD"; } export function formatNumber( @@ -32,7 +30,6 @@ export function formatNumber( }).format(num); } - export function previewStatsIsBarStats( stats: PreviewStatistics, ): stats is BarStatistics { From 67207460375b0b0927c619963bd64ddf27d2ac71 Mon Sep 17 00:00:00 2001 From: Kai Rollmann Date: Mon, 15 Apr 2024 11:36:24 +0200 Subject: [PATCH 3/3] Fix some formatting --- frontend/src/js/preview/Table.tsx | 2 +- frontend/src/js/preview/util.ts | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/frontend/src/js/preview/Table.tsx b/frontend/src/js/preview/Table.tsx index c019a38995..8cd7f64184 100644 --- a/frontend/src/js/preview/Table.tsx +++ b/frontend/src/js/preview/Table.tsx @@ -77,7 +77,7 @@ export default memo(function Table({ key: field.name, render: (value: string | Vector) => { const rendered = renderer(value); - return {rendered}; + return {rendered}; }, }; }), diff --git a/frontend/src/js/preview/util.ts b/frontend/src/js/preview/util.ts index 0b5814a646..e4e777e4c3 100644 --- a/frontend/src/js/preview/util.ts +++ b/frontend/src/js/preview/util.ts @@ -5,14 +5,8 @@ export const NUMBER_TYPES = ["NUMERIC", "INTEGER"]; export const NUMBER_STATISTICS_TYPES = [...NUMBER_TYPES, "MONEY"]; export function currencyFromSymbol(symbol: string): string { - //TODO this is a workaround until the backend sends currency-codes - if (symbol == "€") { - return "EUR"; - } - - if (symbol == "$") { - return "USD"; - } + // TODO: this is a workaround until the backend sends currency-codes + if (symbol === "€") return "EUR"; return "USD"; }