Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React 18 Flickering #684

Open
markmcdowell opened this issue May 13, 2023 · 11 comments
Open

React 18 Flickering #684

markmcdowell opened this issue May 13, 2023 · 11 comments

Comments

@markmcdowell
Copy link
Collaborator

markmcdowell commented May 13, 2023

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 markmcdowell pinned this issue May 13, 2023
@jsnns
Copy link

jsnns commented May 25, 2023

Any updates on this?

@Arnau-Gelonch
Copy link

Has anyone been able to solve it?

@jsnns
Copy link

jsnns commented Jun 12, 2023

@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
  }
}

@avi2d
Copy link

avi2d commented Aug 10, 2023

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

@carterjfulcher
Copy link

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.

@electronicbits
Copy link

electronicbits commented Oct 10, 2023

@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

@pencilcheck
Copy link

pencilcheck commented Jan 23, 2024

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.

@donkey-donkey
Copy link

donkey-donkey commented Mar 1, 2024

@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

@Cr1pter
Copy link

Cr1pter commented Mar 28, 2024

Is there any solution to this in next.js?

@ReCodee
Copy link

ReCodee commented Apr 9, 2024

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
Copy link

ReCodee commented Apr 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

10 participants