Bound high-cardinality chart rendering work

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@netdata/charts",
-  "version": "6.12.4",
+  "version": "6.12.5",
   "description": "Netdata frontend SDK and chart utilities",
   "main": "dist/index.js",
   "module": "dist/es6/index.js",

src/chartLibraries/bars/index.js

@@ -31,20 +31,17 @@ export default (sdk, chart) => {
   const getMinMax = () => chart.getAttribute("getValueRange")(chart)
 
   const render = () => {
-    chartUI.render()
-
     const { hoverX, loaded } = chart.getAttributes()
 
-    if (!loaded) return
+    if (!loaded) return false
 
     const { data } = chart.getPayload()
 
     const row = hoverX ? chart.getClosestRow(hoverX[0]) : data.length - 1
 
     const rowData = data[row]
-    if (!Array.isArray(rowData)) return
+    if (!Array.isArray(rowData)) return chartUI.render()
 
-    chartUI.render()
     const [min, max] = getMinMax()
 
     if (min !== prevMin || max !== prevMax) {
@@ -54,7 +51,9 @@ export default (sdk, chart) => {
     prevMin = min
     prevMax = max
 
+    chartUI.render()
     chartUI.trigger("rendered")
+    return true
   }
 
   const unmount = () => {

src/chartLibraries/d3pie/index.js

@@ -63,11 +63,9 @@ export default (sdk, chart) => {
   const getMinMax = () => chart.getAttribute("getValueRange")(chart)
 
   const render = () => {
-    chartUI.render()
-
     const { hoverX, loaded } = chart.getAttributes()
 
-    if (!pie || !loaded) return
+    if (!pie || !loaded) return false
 
     const { data } = chart.getPayload()
 
@@ -100,8 +98,6 @@ export default (sdk, chart) => {
     prevMin = min
     prevMax = max
 
-    chartUI.render()
-
     pie.options.data.content = values.length
       ? values
       : [
@@ -117,7 +113,9 @@ export default (sdk, chart) => {
       reMake()
     })
 
+    chartUI.render()
     chartUI.trigger("rendered")
+    return true
   }
 
   const unmount = () => {

src/chartLibraries/dygraph/index.js

@@ -18,6 +18,7 @@ import makeHoverX from "./hoverX"
 import makeOverlays from "./overlays"
 import crosshair from "./crosshair"
 import { makeDivergingStackedDataHandler } from "./divergingStack"
+import makeDygraphWithoutLegend from "./makeDygraph"
 
 const touchEvents = ["touchstart", "touchmove", "touchend"]
 
@@ -31,6 +32,18 @@ export default (sdk, chart) => {
   let resizeObserver = null
   let overlays = null
   let executeLatest = null
+  let applyingOptions = false
+
+  const updateOptions = options => {
+    const blockRedraw = applyingOptions
+    applyingOptions = true
+
+    try {
+      return dygraph.updateOptions(options, blockRedraw)
+    } finally {
+      applyingOptions = blockRedraw
+    }
+  }
 
   const mount = element => {
     if (dygraph) return
@@ -47,7 +60,7 @@ export default (sdk, chart) => {
 
     const isEmpty = attributes.outOfLimits || data.length === 0
 
-    dygraph = new Dygraph(element, isEmpty ? [[0]] : data, {
+    dygraph = makeDygraphWithoutLegend(element, isEmpty ? [[0]] : data, {
       // timingName: chart.getId(),
       legend: "never",
       showLabelsOnHighlight: false,
@@ -161,15 +174,15 @@ export default (sdk, chart) => {
       chart.onAttributeChange("theme", (nextTheme, prevTheme) => {
         element.classList.remove(prevTheme)
         element.classList.add(nextTheme)
-        dygraph.updateOptions(makeThemingOptions())
+        updateOptions(makeThemingOptions())
       }),
       chart.onAttributeChange("chartType", () => {
         if (chart.getAttribute("processing")) return
 
-        dygraph.updateOptions(makeChartTypeOptions())
+        updateOptions(makeChartTypeOptions())
       }),
       chart.onAttributeChange("unitsConversionPrefix", () => {
-        dygraph.updateOptions({
+        updateOptions({
           ...makeChartTypeOptions(),
           digitsAfterDecimal:
             chart.getAttribute("unitsConversionFractionDigits")[0] < 0
@@ -180,7 +193,7 @@ export default (sdk, chart) => {
       chart.onAttributeChange("selectedLegendDimensions", () => {
         if (chart.getAttribute("processing")) return
 
-        dygraph.updateOptions({
+        updateOptions({
           ...makeVisibilityOptions(),
           ...makeColorOptions(),
           ...makeChartTypeOptions(),
@@ -189,23 +202,24 @@ export default (sdk, chart) => {
               ? 0
               : chart.getAttribute("unitsConversionFractionDigits")[0],
         })
+        chartUI.render()
       }),
       chart.onAttributeChange("staticValueRange", staticValueRange => {
         if (!staticValueRange) {
-          dygraph.updateOptions({
+          updateOptions({
             valueRange: attributes.getValueRange(chart, { dygraph: true }),
           })
           return
         }
 
         const [min, max] = staticValueRange
-        dygraph.updateOptions({
+        updateOptions({
           valueRange: isHeatmap(attributes.chartType)
             ? [Math.ceil(min), Math.ceil(max)]
             : attributes.getValueRange(chart, { dygraph: true }),
         })
       }),
-      chart.onAttributeChange("timezone", () => dygraph.updateOptions({})),
+      chart.onAttributeChange("timezone", () => updateOptions({})),
     ].filter(Boolean)
 
     overlays.toggle()
@@ -324,9 +338,10 @@ export default (sdk, chart) => {
     const { chartType, includeZero, enabledXAxis, enabledYAxis, yAxisLabelWidth, stepPlot } =
       chart.getAttributes()
 
-    const plotter = stepPlot && chartType === "line"
-      ? plotterByChartType.default
-      : plotterByChartType[chartType] || plotterByChartType.default
+    const plotter =
+      stepPlot && chartType === "line"
+        ? plotterByChartType.default
+        : plotterByChartType[chartType] || plotterByChartType.default
 
     const {
       strokeWidth,
@@ -343,14 +358,14 @@ export default (sdk, chart) => {
     } = optionsByChartType[chartType] || optionsByChartType.default
 
     const yAxisLabelFormatter = makeYAxisLabelFormatter(labels)
-      const yTicker = makeYTicker
-        ? makeYTicker({
-            labels: chart.getVisibleHeatmapIds?.() || chart.getVisibleDimensionIds(),
-            scale: chart.getHeatmapScale?.(),
-            secondsAsTime: chart.getAttribute("secondsAsTime"),
-            units: chart.getVisibleDimensionIds().map(id => chart.getDimensionUnit(id)),
-          })
-        : null
+    const yTicker = makeYTicker
+      ? makeYTicker({
+          labels: chart.getVisibleHeatmapIds?.() || chart.getVisibleDimensionIds(),
+          scale: chart.getHeatmapScale?.(),
+          secondsAsTime: chart.getAttribute("secondsAsTime"),
+          units: chart.getVisibleDimensionIds().map(id => chart.getDimensionUnit(id)),
+        })
+      : null
 
     const { selectedLegendDimensions } = chart.getAttributes()
     const dimensionIds = chart.getPayloadDimensionIds()
@@ -476,14 +491,12 @@ export default (sdk, chart) => {
   const getDygraph = () => dygraph
 
   const render = () => {
-    if (!dygraph) return
+    if (!dygraph) return false
 
     const { highlighting, panning, processing } = chart.getAttributes()
-    if (highlighting || panning || processing) return
+    if (highlighting || panning || processing) return false
 
-    chartUI.render()
-
-    dygraph.updateOptions({
+    updateOptions({
       ...makeDataOptions(),
       ...makeVisibilityOptions(),
       ...makeColorOptions(),
@@ -495,7 +508,9 @@ export default (sdk, chart) => {
       ...makeSparklineOptions(),
     })
 
+    chartUI.render()
     chartUI.trigger("rendered")
+    return true
   }
 
   const getPreceded = () => {

src/chartLibraries/dygraph/index.test.js

@@ -128,6 +128,38 @@ describe("dygraphChart", () => {
     expect(() => instance.render()).not.toThrow()
   })
 
+  it("blocks a nested option redraw while preserving the outer redraw", () => {
+    const { sdk, chart } = makeTestChart({
+      attributes: {
+        highlighting: false,
+        panning: false,
+        processing: false,
+      },
+    })
+
+    chart.getPayload = () => ({
+      data: [[1617946860000, 10]],
+      labels: ["time", "value"],
+    })
+    chart.getDateWindow = () => [1617946860000, 1617947750000]
+    chart.formatXAxis = x => x.toString()
+    chart.getThemeAttribute = () => "#333"
+    chart.getUnitSign = () => ""
+
+    const instance = dygraphChart(sdk, chart)
+    instance.mount(document.createElement("div"))
+    mockDygraph.updateOptions.mockClear()
+    mockDygraph.updateOptions.mockImplementationOnce(() => {
+      chart.updateAttribute("unitsConversionPrefix", ["Ki"])
+    })
+
+    instance.render()
+
+    expect(mockDygraph.updateOptions).toHaveBeenCalledTimes(2)
+    expect(mockDygraph.updateOptions.mock.calls[0][1]).toBe(false)
+    expect(mockDygraph.updateOptions.mock.calls[1][1]).toBe(true)
+  })
+
   it("skips render when highlighting, panning, or processing", () => {
     const { sdk, chart } = makeTestChart({
       attributes: {

src/chartLibraries/dygraph/makeDygraph.js

@@ -0,0 +1,18 @@
+import Dygraph from "dygraphs"
+
+const makeDygraphWithoutLegend = (...args) => {
+  const plugins = Dygraph.PLUGINS
+  const Legend = Dygraph.Plugins?.Legend
+
+  if (!Array.isArray(plugins) || !Legend) return new Dygraph(...args)
+
+  Dygraph.PLUGINS = plugins.filter(Plugin => Plugin !== Legend)
+
+  try {
+    return new Dygraph(...args)
+  } finally {
+    Dygraph.PLUGINS = plugins
+  }
+}
+
+export default makeDygraphWithoutLegend

src/chartLibraries/dygraph/makeDygraph.test.js

@@ -0,0 +1,78 @@
+import Dygraph from "dygraphs"
+import makeDygraphWithoutLegend from "./makeDygraph"
+
+const originalPlugins = Dygraph.PLUGINS
+const instances = []
+
+const createElement = () => {
+  const element = document.createElement("div")
+  Object.defineProperties(element, {
+    clientWidth: { value: 800 },
+    clientHeight: { value: 400 },
+  })
+  element.style.padding = "0px"
+  document.body.appendChild(element)
+  return element
+}
+
+const createDygraph = (options = {}) => {
+  const element = createElement()
+
+  const instance = makeDygraphWithoutLegend(element, [[0, 1]], {
+    labels: ["time", "value"],
+    legend: "never",
+    ...options,
+  })
+
+  instances.push(instance)
+  return instance
+}
+
+describe("makeDygraph without the built-in legend", () => {
+  afterEach(() => {
+    instances.splice(0).forEach(instance => instance.destroy())
+    document.body.replaceChildren()
+    Dygraph.PLUGINS = originalPlugins
+  })
+
+  it("omits only the unused built-in legend plugin", () => {
+    const instance = createDygraph()
+    const activePluginTypes = instance.plugins_.map(({ plugin }) => plugin.constructor)
+
+    expect(activePluginTypes).toEqual(
+      originalPlugins.filter(Plugin => Plugin !== Dygraph.Plugins.Legend)
+    )
+    expect(Dygraph.PLUGINS).toBe(originalPlugins)
+  })
+
+  it("preserves plugins supplied for the chart instance", () => {
+    const customPlugin = {
+      activate: () => ({}),
+    }
+    const instance = createDygraph({ plugins: [customPlugin] })
+
+    expect(instance.plugins_.some(({ plugin }) => plugin === customPlugin)).toBe(true)
+    expect(Dygraph.PLUGINS).toBe(originalPlugins)
+  })
+
+  it("restores the global plugin registry when construction fails", () => {
+    class FailingPlugin {
+      activate() {
+        throw new Error("plugin failed")
+      }
+    }
+
+    const configuredPlugins = [...originalPlugins, FailingPlugin]
+    Dygraph.PLUGINS = configuredPlugins
+
+    const element = createElement()
+
+    expect(() =>
+      makeDygraphWithoutLegend(element, [[0, 1]], {
+        labels: ["time", "value"],
+        legend: "never",
+      })
+    ).toThrow("plugin failed")
+    expect(Dygraph.PLUGINS).toBe(configuredPlugins)
+  })
+})