lightweight-charts icon indicating copy to clipboard operation
lightweight-charts copied to clipboard

React Advanced Chart Example in Documentation has bug when removing chart during resizing

Open patrickcook28 opened this issue 1 year ago • 1 comments

Lightweight Charts™ Version: 4.0.1

Steps/code to reproduce:

Copy advanced react example from Lightweight Chart docs

export const ChartContainer = forwardRef((props, ref) => {
  const { children, container, layout, ...rest } = props;

  const chartApiRef = useRef({
    api() {
      if (!this._api) {
        this._api = createChart(container, {
          ...rest,
          ...layout,
          width: container.clientWidth,
          height: window.innerHeight * 0.8,
        });
        this._api.timeScale().fitContent();
      }
      return this._api;
    },
    free() {
      if (this._api) {
        this._api.remove(); // error occurs here
      }
    },
  });
  ...


export const Series = forwardRef((props, ref) => {
  const parent = useContext(ChartContext);
  const context = useRef({
    api() {
      if (!this._api) {
        const { children, data, ...rest } = props;
        this._api = parent.api().addCandlestickSeries(rest);
        this._api.setData(data);
      }
      return this._api;
    },
    free() {
      if (this._api) {
        parent.free(); // error occurs here
      }
    },
  });
  ...

Actual behavior:

When copying the Advanced React chart example that uses Refs and splits the Chart, Container, and Series into individual components, it doesn't work. An error is thrown when trying to remove the chart.

In development mode, React will run all useEffects and useLayoutEffects, then run their clean-up functions, then re-run the effects. There are some bugs in the clean-up functions which caused the example to not run properly.

The error occurred when chart.remove() was called in the useLayoutEffect hook that was responsible for handling resizing of the chart.

Expected behavior:

The example should work out of the box.

The example posted on the website should include proper clean-up functions. The solution was to add this._api = null; in the free() functions for both the ChartContainer and Series components.

patrickcook28 avatar Sep 25 '23 15:09 patrickcook28

I would use this for Chart

if (this._api) {
  this._api.remove();
  this._api = undefined;
}

and this for Series

if (this._api) {
  parent._api && parent.api().removeSeries(this._api);
  this._api = undefined;
}

mishurov avatar Nov 26 '23 10:11 mishurov