fix: use Map and memoization for consistent Mosaic (#692)

giraffe/package.json

@@ -1,6 +1,6 @@
 {
  "name": "@influxdata/giraffe",
- "version": "2.18.11",
+ "version": "2.19.0",
  "main": "dist/index.js",
  "module": "src/index.js",
  "license": "MIT",
@@ -47,7 +47,6 @@
  "@types/d3-scale": "^2.1.1",
  "@types/d3-shape": "^1.3.1",
  "@types/jest": "^26.0.19",
- "@types/memoize-one": "^4.1.1",
  "@types/react": "^16.8.3",
  "@types/react-dom": "^16.8.2",
  "@types/resize-observer-browser": "^0.1.5",
@@ -81,7 +80,7 @@
  "leaflet": "^1.6.0",
  "leaflet-ant-path": "^1.3.0",
  "leaflet.markercluster": "^1.4.1",
- "memoize-one": "^5.0.2",
+ "memoize-one": "^6.0.0",
  "node-sass": "^4.14.1",
  "papaparse": "^5.3.0",
  "prettier": "^1.19.1",

giraffe/src/components/TableGraphTransform.tsx

@@ -21,25 +21,7 @@ interface Props {
  children: (transformedDataBundle: TransformTableDataReturnType) => JSX.Element
 }
 
-const areFormatPropertiesEqual = (
- prevProperties: Props,
- newProperties: Props
-) => {
- const formatProps = ['tableOptions', 'fieldOptions', 'timeFormat', 'sort']
- if (!prevProperties.properties) {
- return false
- }
- const propsEqual = formatProps.every(k =>
- isEqual(prevProperties.properties[k], newProperties.properties[k])
- )
-
- return propsEqual
-}
-
-const memoizedTableTransform = memoizeOne(
- transformTableData,
- areFormatPropertiesEqual
-)
+const memoizedTableTransform = memoizeOne(transformTableData, isEqual)
 
 export const TableGraphTransform: FunctionComponent<Props> = (props: Props) => {
  const {properties, data, dataTypes, sortOptions} = props

giraffe/src/transforms/mosaic.ts

@@ -1,19 +1,30 @@
-import {newTable} from '../utils/newTable'
+import memoizeOne from 'memoize-one'
 import {MosaicLayerSpec, Table} from '../types'
 import {DISPLAY_NAME, FILL, SERIES, X_MAX, X_MIN} from '../constants/columnKeys'
-import {createGroupIDColumn} from './'
+import {isEqual} from '../utils/isEqual'
+import {newTable} from '../utils/newTable'
 import {resolveDomain} from '../utils/resolveDomain'
-import {getNominalColorScale} from './'
+import {createGroupIDColumn, getNominalColorScale} from './'
+
+const memoizedSortTimeStamps = memoizeOne(
+ (timeStamps: Iterable<number>) => [...timeStamps].sort(),
+ isEqual
+)
+
+const memoizedSortDataMapKeys = memoizeOne(
+ (dataMapKeys: Iterable<string>) => [...dataMapKeys].sort(),
+ isEqual
+)
 
 export const mosaicTransform = (
  inputTable: Table,
  xColumnKey: string,
- yColumnKeys: string[],
- yLabelColumns: string[],
+ yColumnKeys: Array<string>,
+ yLabelColumns: Array<string>,
  yLabelColumnSeparator: string,
- xDomain: number[],
- fillColKeys: string[],
- colors: string[]
+ xDomain: Array<number>,
+ fillColKeys: Array<string>,
+ colors: Array<string>
 ): MosaicLayerSpec => {
  const [fillColumn, fillColumnMap] = createGroupIDColumn(
  inputTable,
@@ -34,35 +45,36 @@ export const mosaicTransform = (
  : columnName,
  ''
  )
- const yInputCols = {}
+ const yInputCols = new Map()
  if (Array.isArray(yColumnKeys)) {
  yColumnKeys.forEach(columnKey => {
- const column = inputTable.getColumn(columnKey, 'string')
- yInputCols[columnKey] = column
+ if (columnKey) {
+ const column = inputTable.getColumn(columnKey, 'string')
+ yInputCols.set(columnKey, column)
+ }
  })
  }
 
- // Mosaic can only have one column as the fill value,
- //  always the first fill column key
+ // Mosaic can have only one column as the fill value:
+ // always the first fill column key
  const valueKey = fillColumnMap.columnKeys[0]
 
- const timeStampMap = {}
-
+ const timeStampMap = new Map()
  for (let i = 0; i < inputTable.length; i++) {
  const yColumnTick = Array.isArray(yColumnKeys)
  ? yColumnKeys.reduce((combinedValue, key) => {
  let value = ''
- if (yInputCols[key]) {
- value = yInputCols[key][i]
+ if (yInputCols.has(key) && Array.isArray(yInputCols.get(key))) {
+ value = yInputCols.get(key)[i]
  }
  return `${combinedValue}${value}`
  }, '')
  : ''
 
  const yTickLabel = labelColumns.reduce((combinedValue, key) => {
  let value = ''
- if (yInputCols[key]) {
- value = yInputCols[key][i]
+ if (yInputCols.has(key)) {
+ value = yInputCols.get(key)[i]
  }
  return combinedValue
  ? `${combinedValue}${yLabelColumnSeparator}${value}`
@@ -72,17 +84,17 @@ export const mosaicTransform = (
  const currentX = xInputCol[i]
  const currentFillValue = fillColumnMap.mappings[fillColumn[i]][valueKey]
 
- if (!timeStampMap[currentX]) {
- timeStampMap[currentX] = []
+ if (!timeStampMap.has(currentX)) {
+ timeStampMap.set(currentX, [])
  }
- timeStampMap[currentX].push({
+ timeStampMap.get(currentX).push({
  yTickLabel,
  yColumnTick,
  fill: currentFillValue,
  })
  }
 
- const sortedTimeStamps = Object.keys(timeStampMap).sort()
+ const sortedTimeStamps = memoizedSortTimeStamps([...timeStampMap.keys()])
  let tableLength = 0
 
  /*
@@ -97,34 +109,33 @@ export const mosaicTransform = (
  yTickLabel: string,
  }
  */
- const dataMap = {}
+ const dataMap = new Map()
 
  sortedTimeStamps.forEach(timeStamp => {
- timeStampMap[timeStamp].forEach(data => {
- if (!dataMap[data.yColumnTick]) {
- dataMap[data.yColumnTick] = {
- xMin: [Number(timeStamp)],
- xMax: [Number(timeStamp)],
+ timeStampMap.get(timeStamp).forEach(data => {
+ if (!dataMap.has(data.yColumnTick)) {
+ dataMap.set(data.yColumnTick, {
+ xMin: [timeStamp],
+ xMax: [timeStamp],
  fill: [data.fill],
  series: [data.yColumnTick],
  displayedColumns: [data.yTickLabel],
  yTickLabel: data.yTickLabel,
- }
+ })
  tableLength += 1
  } else {
- const prevMaxIndex = dataMap[data.yColumnTick].xMax.length - 1
- const prevFill =
- dataMap[data.yColumnTick].fill[
- dataMap[data.yColumnTick].fill.length - 1
- ]
+ const prevMaxIndex = dataMap.get(data.yColumnTick).xMax.length - 1
+ const prevFill = dataMap.get(data.yColumnTick).fill[
+ dataMap.get(data.yColumnTick).fill.length - 1
+ ]
 
- dataMap[data.yColumnTick].xMax[prevMaxIndex] = Number(timeStamp)
+ dataMap.get(data.yColumnTick).xMax[prevMaxIndex] = timeStamp
  if (prevFill !== data.fill) {
- dataMap[data.yColumnTick].xMin.push(Number(timeStamp))
- dataMap[data.yColumnTick].xMax.push(Number(timeStamp))
- dataMap[data.yColumnTick].fill.push(data.fill)
- dataMap[data.yColumnTick].series.push(data.yColumnTick)
- dataMap[data.yColumnTick].displayedColumns.push(data.yTickLabel)
+ dataMap.get(data.yColumnTick).xMin.push(timeStamp)
+ dataMap.get(data.yColumnTick).xMax.push(timeStamp)
+ dataMap.get(data.yColumnTick).fill.push(data.fill)
+ dataMap.get(data.yColumnTick).series.push(data.yColumnTick)
+ dataMap.get(data.yColumnTick).displayedColumns.push(data.yTickLabel)
  tableLength += 1
  }
  }
@@ -139,24 +150,21 @@ export const mosaicTransform = (
  const yTicks = []
  const ySeries = []
 
- for (const key in dataMap) {
- //combine all series into the proper shape
- xMinData = xMinData.concat(dataMap[key].xMin)
- xMaxData = xMaxData.concat(dataMap[key].xMax)
- fillData = fillData.concat(dataMap[key].fill)
- seriesData = seriesData.concat(dataMap[key].series)
+ const sortedDataMapKeys = memoizedSortDataMapKeys([...dataMap.keys()])
+
+ sortedDataMapKeys.forEach(key => {
+ // combine all series into the proper shape
+ xMinData = xMinData.concat(dataMap.get(key).xMin)
+ xMaxData = xMaxData.concat(dataMap.get(key).xMax)
+ fillData = fillData.concat(dataMap.get(key).fill)
+ seriesData = seriesData.concat(dataMap.get(key).series)
  displayedColumnsData = displayedColumnsData.concat(
- dataMap[key].displayedColumns
+ dataMap.get(key).displayedColumns
  )
  ySeries.push(key)
- yTicks.push(dataMap[key].yTickLabel)
- }
- /*
- xMin (start time) | xMax (end time) | Value Category | host | cpu
- -------------------------------------------------------------------
- 1554308748000 | 1554308758000 | 'eenie' | "a" | 1
- 1554308748000 | 1554308758000 | 'mo' | "b" | 2
- */
+ yTicks.push(dataMap.get(key).yTickLabel)
+ })
+
  const table = newTable(tableLength)
  .addColumn(X_MIN, 'system', 'number', xMinData)
  .addColumn(X_MAX, 'system', 'number', xMaxData)

stories/package.json

@@ -1,6 +1,6 @@
 {
  "name": "@influxdata/giraffe-stories",
- "version": "2.18.11",
+ "version": "2.19.0",
  "license": "MIT",
  "scripts": {
  "lint": "eslint '{src,../giraffe/src}/**/*.{ts,tsx}'",
@@ -18,7 +18,6 @@
  "@storybook/core": "5",
  "@storybook/react": "^5.0.10",
  "@storybook/storybook-deployer": "^2.8.1",
- "@types/memoize-one": "^4.1.1",
  "@types/node-sass": "^4.11.0",
  "@types/react": "^16.8.3",
  "@types/react-dom": "^16.8.2",
@@ -34,7 +33,7 @@
  "eslint-plugin-react": "^7.12.4",
  "file-loader": "^3.0.1",
  "intl-dateformat": "^0.1.1",
- "memoize-one": "^5.0.2",
+ "memoize-one": "^6.0.0",
  "prettier": "^1.19.1",
  "react-virtualized-auto-sizer": "^1.0.2",
  "resolve-url-loader": "^3.0.1",

yarn.lock

@@ -2467,11 +2467,6 @@
  resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.6.tgz#f4c7ec43e81b319a9815115031709f26987891f0"
  integrity sha512-3c+yGKvVP5Y9TYBEibGNR+kLtijnj7mYrXRg+WpFb2X9xm04g/DXYkfg4hmzJQosc9snFNUPkbYIhu+KAm6jJw==
 
-"@types/memoize-one@^4.1.1":
- version "4.1.1"
- resolved "https://registry.yarnpkg.com/@types/memoize-one/-/memoize-one-4.1.1.tgz#41dd138a4335b5041f7d8fc038f9d593d88b3369"
- integrity sha512-+9djKUUn8hOyktLCfCy4hLaIPgDNovaU36fsnZe9trFHr6ddlbIn2q0SEsnkCkNR+pBWEU440Molz/+Mpyf+gQ==
-
 "@types/minimatch@*":
  version "3.0.3"
  resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d"
@@ -9793,11 +9788,16 @@ mem@^4.0.0:
  mimic-fn "^2.0.0"
  p-is-promise "^2.0.0"
 
-memoize-one@^5.0.0, memoize-one@^5.0.2:
+memoize-one@^5.0.0:
  version "5.0.4"
  resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.0.4.tgz#005928aced5c43d890a4dfab18ca908b0ec92cbc"
  integrity sha512-P0z5IeAH6qHHGkJIXWw0xC2HNEgkx/9uWWBQw64FJj3/ol14VYdfVGWWr0fXfjhhv3TKVIqUq65os6O4GUNksA==
 
+memoize-one@^6.0.0:
+ version "6.0.0"
+ resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-6.0.0.tgz#b2591b871ed82948aee4727dc6abceeeac8c1045"
+ integrity sha512-rkpe71W0N0c0Xz6QD0eJETuWAJGnJ9afsl1srmwPrI+yBCkge5EycXXbYRyvL29zZVUWQCY7InPRCv3GDXuZNw==
+
 memoizerific@^1.11.3:
  version "1.11.3"
  resolved "https://registry.yarnpkg.com/memoizerific/-/memoizerific-1.11.3.tgz#7c87a4646444c32d75438570905f2dbd1b1a805a"