react-financial-charts icon indicating copy to clipboard operation
react-financial-charts copied to clipboard

React 18 Flickering

Open markmcdowell opened this issue 1 year ago • 11 comments

When running under React 18 the current version of the charts is flickering on mouse wheel operations. This appears to be due to React 18 batching updates originating from native event handlers. See https://react.dev/blog/2022/03/08/react-18-upgrade-guide#automatic-batching

We've tried using the recommend method of FlushSync to avoid the batching, whilst this does remove the flickering, it also produces an unacceptable level of performance and jerk.

The only workaround at the moment is to use ReactDom.render instead of ReactDom.createRoot().render, this will mean the same behaviour as React 17 but with a warning in the console.

markmcdowell avatar May 13 '23 22:05 markmcdowell

Any updates on this?

jsnns avatar May 25 '23 03:05 jsnns

Has anyone been able to solve it?

Arnau-Gelonch avatar Jun 10 '23 13:06 Arnau-Gelonch

@Arnau-Gelonch I found a (very suboptimal) work-around for now. It's not perfect:

  • struggles to handle resizing
  • miserable for performance (very noticeable on mobile) if you have a busy page
  • interactions are still a big laggy but doesn't flash which is an improvement
class StockChart extends React.Component<StockChartProps> {
  idFromProps(): string {
    return `candle-chart-${this.randomId}`;
  }

  componentDidUpdate(): void {
    if (this.props.data.length === 0) {
      return;
    }

    // if there is already a chart rendered, remove it
    const element = document.getElementById(this.idFromProps());
    if (this.hasRendered && element) {
      ReactDOM.unmountComponentAtNode(element);
      this.hasRendered = false;
    }
    if (!this.hasRendered) {
      ReactDOM.render(this.internalRender(), document.getElementById(this.idFromProps()));
      this.hasRendered = true;
    }
  }

  componentDidMount(): void {
    this.componentDidUpdate();
  }

  componentWillUnmount(): void {
    const element = document.getElementById(this.idFromProps());
    if (this.hasRendered && element) {
      ReactDOM.unmountComponentAtNode(element);
    }
  }

  public render() {
    // use ReactDom.render to render the component
    return <div id={this.idFromProps()} />;
  }

  public internalRender() {
    // render your chart
  }
}

jsnns avatar Jun 12 '23 04:06 jsnns

Leave this.clearThreeCanvas() only in getSnapshotBeforeUpdate and remove it from other places

avi2d avatar Aug 10 '23 20:08 avi2d

Are there any updates on this? My company is looking to use this library in an upcoming product. We could possibly devote dev time to fixing.

carterjfulcher avatar Oct 09 '23 22:10 carterjfulcher

@carterjfulcher I would've thought this is already fixed, there is no flickering anymore (under React 17 rendering method) However, I do notice the chart is not displayed when panning vertically. It does work horizontally. This can be checked on the current stories. I spent a few hours debugging the code, inspecting basically the draw functions in the ChartCanvas class but have not been able to identify the issue yet, as they "seem" to be ok to me so far. Anyone else having this issue ? Any idea ?

issue-charts

electronicbits avatar Oct 10 '23 01:10 electronicbits

On React 18, panning and zooming still flickers.

Tried avi2d method, doesn't work for me.

By changing

    shouldComponentUpdate() {
        return false;
    }

in "node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@reac t-financial-charts/core/lib/ChartCanvas.js" It makes the flickering a lot less, but there are still flickering at random intervals.

pencilcheck avatar Jan 23 '24 16:01 pencilcheck

@carterjfulcher I would've thought this is already fixed, there is no flickering anymore (under React 17 rendering method) However, I do notice the chart is not displayed when panning vertically. It does work horizontally. This can be checked on the current stories. I spent a few hours debugging the code, inspecting basically the draw functions in the ChartCanvas class but have not been able to identify the issue yet, as they "seem" to be ok to me so far. Anyone else having this issue ? Any idea ?

this is exactly what we are experiencing

donkey-donkey avatar Mar 01 '24 01:03 donkey-donkey

Is there any solution to this in next.js?

Cr1pter avatar Mar 28 '24 10:03 Cr1pter

Leave this.clearThreeCanvas() only in getSnapshotBeforeUpdate and remove it from other places

This fixed the problem, Does it have any consequences? If not why is someone not creating a PR as this fixes the issue?

ReCodee avatar Apr 09 '24 17:04 ReCodee

Leave this.clearThreeCanvas() only in getSnapshotBeforeUpdate and remove it from other places

@Cr1pter I resolved the issue in next.js with this suggestion, You can do these changes in the ChartCanvas.js present in the module.

ReCodee avatar Apr 09 '24 18:04 ReCodee