From 2c636b65a1ed437b1c0e39154a531fcda12013bc Mon Sep 17 00:00:00 2001 From: Venryx Date: Thu, 12 Nov 2020 17:37:07 -0800 Subject: [PATCH] * Updated uplot dependency. * Tried to add workaround for ReactJS memory-leak issue. (where variables referenced in useEffect cleanup function can leak, by means of the cleanup-function ref being retained!) [see: https://github.com/facebook/react/issues/18790#issuecomment-726394247] Workaround didn't seem to work for every case though, so I fell back to a userland fix. (setting array.length to 0, for the old passed "data" array, so that even though leaked reference remains, it has no significant memory cost) --- Dist/UI/UPlot.d.ts | 2 +- Dist/UI/UPlot.js | 43 +++++++++++++++++++++++++++++++------------ Source/UI/UPlot.tsx | 40 ++++++++++++++++++++++++++++++---------- package-lock.json | 6 +++--- package.json | 2 +- 5 files changed, 66 insertions(+), 27 deletions(-) diff --git a/Dist/UI/UPlot.d.ts b/Dist/UI/UPlot.d.ts index dba7269..00404f7 100644 --- a/Dist/UI/UPlot.d.ts +++ b/Dist/UI/UPlot.d.ts @@ -1,7 +1,7 @@ import React from "react"; import uPlot from "uplot"; import "uplot/dist/uPlot.min.css"; -export declare const UPlot: React.MemoExoticComponent<(p: { +export declare const UPlot: React.MemoExoticComponent<(props: { divRef?: React.RefObject; chartRef?: React.MutableRefObject; options: uPlot.Options; diff --git a/Dist/UI/UPlot.js b/Dist/UI/UPlot.js index 998e568..9262b7f 100644 --- a/Dist/UI/UPlot.js +++ b/Dist/UI/UPlot.js @@ -2,24 +2,43 @@ import React, { useEffect, useRef } from "react"; import uPlot from "uplot"; import "uplot/dist/uPlot.min.css"; import { Assert, E } from "../Utils/FromJSVE"; -export const UPlot = React.memo((p) => { - var _a; - const divRef = (_a = p.divRef) !== null && _a !== void 0 ? _a : useRef(null); - Assert(p.data == null || p.data.every(a => { var _a; return a.length == ((_a = p.data[0]) === null || _a === void 0 ? void 0 : _a.length); }), () => `All data-arrays must have the same length. Got lengths: ${p.data.map(a => a.length).join(",")}`); +export const UPlot = React.memo((props) => { + let { divRef, chartRef, options, data, placeLegendBelowContainer } = props; // annoying, but needed to prevent memory leak (see: https://github.com/facebook/react/issues/18790#issuecomment-726394247) + divRef = divRef !== null && divRef !== void 0 ? divRef : useRef(null); + Assert(data == null || data.every(a => { var _a; return a.length == ((_a = data[0]) === null || _a === void 0 ? void 0 : _a.length); }), () => `All data-arrays must have the same length. Got lengths: ${data.map(a => a.length).join(",")}`); + const deps = [data, options, chartRef, divRef]; useEffect(() => { //debugger; - const chart = new uPlot(p.options, p.data, divRef.current); - if (p.chartRef) - p.chartRef.current = chart; + let chart = new uPlot(options, data, divRef.current); + if (chartRef) + chartRef.current = chart; return () => { - if (p.chartRef) - p.chartRef.current = null; + if (chartRef) + chartRef.current = null; chart.destroy(); + chart = null; + // this is "improper", but it's the only easy way I know to solve all versions of the leak reliably! (ie. not knowing react-tree above this comp) + // (Note: In debug mode, the memory leak occurs even with this hack, through "_debugOwner" prop. In prod mode, the hack below is... almost sufficient, though. [updateQueue go away!]) + try { + options = null; + data = null; + divRef = null; + chartRef = null; + deps.length = 0; + props.options = null; + props.data = null; + } + catch (ex) { + // typically, props object is frozen, so hack won't work -- this catch-block ignores the error + // hack is still useful in my own projects, where I disable Object.freeze: https://stackoverflow.com/a/39253443/2441655 + } }; - }, [p.data, p.options, p.chartRef, divRef]); + //}, [data, options, chartRef, divRef]); + }, deps); + //}, [p.options, p.chartRef, divRef]); const randomID = `id_${Math.random().toString().replace(".", "")}`; - const div = (React.createElement("div", { ref: divRef, style: E({ width: "100%", height: "100%" }, p.placeLegendBelowContainer && { height: "calc(100% + 33px)", pointerEvents: "none" }) })); - if (p.placeLegendBelowContainer) { + const div = (React.createElement("div", { ref: divRef, style: E({ width: "100%", height: "100%" }, placeLegendBelowContainer && { height: "calc(100% + 33px)", pointerEvents: "none" }) })); + if (placeLegendBelowContainer) { return (React.createElement("div", { id: randomID, style: { width: "100%", height: "100%" } }, React.createElement("style", null, ` #${randomID} .u-wrap { diff --git a/Source/UI/UPlot.tsx b/Source/UI/UPlot.tsx index 9b32d19..217383f 100644 --- a/Source/UI/UPlot.tsx +++ b/Source/UI/UPlot.tsx @@ -3,33 +3,53 @@ import uPlot from "uplot"; import "uplot/dist/uPlot.min.css"; import {Assert, E} from "../Utils/FromJSVE"; -export const UPlot = React.memo((p: { +export const UPlot = React.memo((props: { divRef?: React.RefObject, chartRef?: React.MutableRefObject, options: uPlot.Options, data: uPlot.AlignedData, placeLegendBelowContainer?: boolean, })=>{ - const divRef = p.divRef ?? useRef(null); - Assert(p.data == null || p.data.every(a=>a.length == p.data[0]?.length), ()=>`All data-arrays must have the same length. Got lengths: ${p.data.map(a=>a.length).join(",")}`); + let {divRef, chartRef, options, data, placeLegendBelowContainer} = props; // annoying, but needed to prevent memory leak (see: https://github.com/facebook/react/issues/18790#issuecomment-726394247) + divRef = divRef ?? useRef(null); + Assert(data == null || data.every(a=>a.length == data[0]?.length), ()=>`All data-arrays must have the same length. Got lengths: ${data.map(a=>a.length).join(",")}`); + const deps = [data, options, chartRef, divRef]; useEffect(()=>{ //debugger; - const chart = new uPlot(p.options, p.data, divRef.current!); - if (p.chartRef) p.chartRef.current = chart; + let chart: uPlot|null = new uPlot(options, data, divRef!.current!); + if (chartRef) chartRef.current = chart; return ()=>{ - if (p.chartRef) p.chartRef.current = null; - chart.destroy(); + if (chartRef) chartRef.current = null; + chart!.destroy(); + chart = null; + + // this is "improper", but it's the only easy way I know to solve all versions of the leak reliably! (ie. not knowing react-tree above this comp) + // (Note: In debug mode, the memory leak occurs even with this hack, through "_debugOwner" prop. In prod mode, the hack below is... almost sufficient, though. [updateQueue go away!]) + try { + options = null as any; + data = null as any; + divRef = null as any; + chartRef = null as any; + deps.length = 0; + props.options = null as any; + props.data = null as any; + } catch (ex) { + // typically, props object is frozen, so hack won't work -- this catch-block ignores the error + // hack is still useful in my own projects, where I disable Object.freeze: https://stackoverflow.com/a/39253443/2441655 + } }; - }, [p.data, p.options, p.chartRef, divRef]); + //}, [data, options, chartRef, divRef]); + }, deps); + //}, [p.options, p.chartRef, divRef]); const randomID = `id_${Math.random().toString().replace(".", "")}`; const div = (
); - if (p.placeLegendBelowContainer) { + if (placeLegendBelowContainer) { return (