I believe you are attaching multiple event listeners. Each time the Page1 component re-renders, it attaches a new set of event listeners without cleaning up the old ones. What causes a re-render? State changes. That's why you are only seeing the issue once you add useState
and setState
.
You can verify this by checking the logs and noticing this not so helpful error:
Warning: Can't perform a React state update on an unmounted component.
This is a no-op, but it indicates a memory leak in your application.
To fix, cancel all subscriptions and asynchronous tasks in a useEffect
cleanup function.
in Page1 (at App.js:13)
You'll need to modify the code related to attaching/detaching event listeners to avoid this. I'm not familiar with Billboard so I can only tell you where the problem is, not the exact spot to fix it.
My gut says its the Chart.jsx here:
renderChart = () => {
console.log('render chart');
const { current } = this.chartRef;
if (current !== null) {
this.chartInstance = bb.generate({
...this.props.options,
bindto: this.chartRef.current
});
}
};
Updated with full solution
I was correct in that the Chart.jsx is where the problem lies.
Listeners should be attached when the DOM is created and removed with the DOM is destroyed. You were not wrong when you first thought to use the React Lifecycles for this chore, however I find the Callback References to be more useful, especially when some of the DOM may be destroyed or created during update cycles (you do not have this problem).
Callback References can be tricky, do not use functions that get recreated each render (trust me its a headache). Callback References are called for two reasons. First, the DOM has been created and to hand you a reference to the element. Second, the DOM has been destroyed, so time to clean up. If React senses a change in the Callback Reference (i.e. you give it a new one) it will tell the first Callback Reference to clean up, and the second Callback Reference to initialize (this is the headache I mentioned). You can avoid this by using an instance method.
// Bind to 'this' otherwise 'this' is lost
setChartRef = (ref) => {
// Remove listeners
if (!ref) {
console.log('no reference');
this.chartRef.current = null;
this.destroy();
}
// Add listeners
else {
console.log('new reference');
this.chartRef.current = ref;
this.createChart();
}
};
Next piece of the puzzle, you only want to call bb.generate
one time. This was causing multiple listeners to be created. So I've simplified and renamed your renderChart
to createChart
createChart = () => {
this.chartInstance = bb.generate({
...this.props.options,
bindto: this.chartRef.current
});
};
Finally, none of the lifecycle methods are necessary because Callback Reference tell us exactly when to create the chart and when to destroy it. You may be wondering what about resizing the chart? Seems like that is taken care of automatically? I could be missing something here, but in the event you need to update the chart, use this.chartInstance
in an update lifecycle method.
My full modifications here:
