Skip to content

Commit

Permalink
* Updated uplot dependency.
Browse files Browse the repository at this point in the history
* 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: facebook/react#18790 (comment)]

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)
  • Loading branch information
Venryx committed Nov 13, 2020
1 parent b4a2088 commit 2c636b6
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 27 deletions.
2 changes: 1 addition & 1 deletion Dist/UI/UPlot.d.ts
Original file line number Diff line number Diff line change
@@ -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<HTMLDivElement>;
chartRef?: React.MutableRefObject<uPlot | null>;
options: uPlot.Options;
Expand Down
43 changes: 31 additions & 12 deletions Dist/UI/UPlot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 30 additions & 10 deletions Source/UI/UPlot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLDivElement>, chartRef?: React.MutableRefObject<uPlot|null>,
options: uPlot.Options, data: uPlot.AlignedData,
placeLegendBelowContainer?: boolean,
})=>{
const divRef = p.divRef ?? useRef<HTMLDivElement>(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<HTMLDivElement>(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 = (
<div ref={divRef} style={E(
{width: "100%", height: "100%"},
p.placeLegendBelowContainer && {height: "calc(100% + 33px)", pointerEvents: "none"} as const,
placeLegendBelowContainer && {height: "calc(100% + 33px)", pointerEvents: "none"} as const,
)}/>
);
if (p.placeLegendBelowContainer) {
if (placeLegendBelowContainer) {
return (
<div id={randomID} style={{width: "100%", height: "100%"}}>
<style>{`
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"react": "^16.0.0",
"react-dom": "^16.0.0",
"typescript": "^4.0.3",
"uplot": "^1.0.0"
"uplot": "^1.3.0"
},
"dependencies": {}
}

0 comments on commit 2c636b6

Please sign in to comment.