Skip to content

DOM element provided is null or undefined #199

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

Closed
lavor opened this issue Aug 28, 2020 · 3 comments · Fixed by #200
Closed

DOM element provided is null or undefined #199

lavor opened this issue Aug 28, 2020 · 3 comments · Fixed by #200

Comments

@lavor
Copy link

lavor commented Aug 28, 2020

Hi, I am still getting this error in some circumstances.

[email protected]
[email protected]

I reported this error in the closed issue #52 two months ago. But I did not get any reaction, so I am opening a new issue.

I tried to debug my situation and come to finding that library is not checking if a component is mounted in promise handlers in updatePlotly function.

Short story
Hence, whenever the component mounts and in next moment it unmounts (before all promises in updatePlotly resolves), the syncWindowResize will call Plotly.Plots.resize with null (this.el) as an argument.

What happened in my situation in details (I've put some checkpoints in comments in bellow snippet):

updatePlotly(shouldInvokeResizeHandler, figureCallbackFunction, shouldAttachUpdateEvents) {
// updatePlotly: checkpoint 1
      this.p = this.p
        .then(() => {
          if (!this.el) {
            let error;
            if (this.unmounting) {
              error = new Error('Component is unmounting');
              error.reason = 'unmounting';
            } else {
              error = new Error('Missing element reference');
            }
            throw error;
          }
// updatePlotly: checkpoint 2
          return Plotly.react(this.el, {
            data: this.props.data,
            layout: this.props.layout,
            config: this.props.config,
            frames: this.props.frames,
          });
        })
        .then(() => this.syncWindowResize(shouldInvokeResizeHandler)  // updatePlotly: checkpoint 3)
        .then(this.syncEventHandlers)
        .then(() => this.figureCallback(figureCallbackFunction))
        .then(shouldAttachUpdateEvents ? this.attachUpdateEvents : () => {})
        .catch(err => {
          if (err.reason === 'unmounting') {
            return;
          }
          console.error('Error while plotting:', err); // eslint-disable-line no-console
          if (this.props.onError) {
            this.props.onError(err);
          }
        });
  1. getRef sets ref correctly
  2. componentDidMount
  3. updatePlotly - updatePlotly: checkpoint 1
  4. updatePlotly - updatePlotly: checkpoint 2
  5. componentWillUnmount
  6. getRef unmounting sets ref to null (see https://reactjs.org/docs/refs-and-the-dom.html#callback-refs)
  7. updatePlotly - updatePlotly: checkpoint 3
  8. syncWindowResize calls Plotly.Plots.resize with argument this.el, which was set to null (step 6) and that results in Error while plotting: Error: DOM element provided is null or undefined.

Possible solution
Do not call things in promise then handler when unmounting:

// ...
 .then(() => {
  if(!this.unmounting) {
    return this.syncWindowResize(shouldInvokeResizeHandler))
  }
}
// ...

Alternative solution
Do not use this.unmountig at all and implement solution with cancelable promises:
https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html

@alexcjohnson
Copy link
Collaborator

Thanks for bringing this up @lavor

Your first solution seems reasonable. AFAICT the whole promise chain after Plotly.react is synchronous, so in addition this could likely all be simplified a good deal, something like:

this.p = this.p
    .then(() => {
        if (this.unmounting) {
            return;
        }
        if (!this.el) {
            throw new Error('Missing element reference');
        }
        return Plotly.react(this.el, {
            data: this.props.data,
            layout: this.props.layout,
            config: this.props.config,
            frames: this.props.frames,
          });
    })
    .then(() => {
        if (this.unmounting) {
            return;
        }
        this.syncWindowResize(shouldInvokeResizeHandler);
        this.syncEventHandlers()
        this.figureCallback(figureCallbackFunction);
        if (shouldAttachUpdateEvents) {
            this.attachUpdateEvents();
        }
    })
    .catch(err => {
        if (this.props.onError) {
          this.props.onError(err);
        }
        // TODO should we have else throw(err) here?
    });

Seem plausible? Either way, this isn't a problem that's come up in our own usage of this package so it's not a high priority for Plotly folks to fix, but if you'd like to make a PR this we'd be more than happy to review and incorporate it!

@lavor
Copy link
Author

lavor commented Sep 2, 2020

Thanks @alexcjohnson,

yes your solution is also correctly working in my environment.
Hence, I created PR for it.
This issue can be closed after merge.

@dmt0 dmt0 closed this as completed in #200 Sep 11, 2020
@eric-burel
Copy link

Hi guys, I am hitting the same error message when running Storyshots (Jest snapshots for Storybook). I don't really get the proposed solution, is there a palliative I can use within React?

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

Successfully merging a pull request may close this issue.

3 participants