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

Hot reload removing portal node on storybook. #750

Closed
wants to merge 1 commit into from

Conversation

bertho-zero
Copy link

In a storybook the hot reload removes the parent before the .removeChild, which makes this mistake once in two:
Can not read property 'removeChild' of null

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.621% when pulling 6ccb9b0 on bertho-zero:patch-1 into a2838bb on reactjs:master.

@bertho-zero
Copy link
Author

Probably related to facebook/react#14811

@diasbruno
Copy link
Collaborator

Hi @bertho-zero, not sure if it is a good idea. Maybe this can hide bugs when removing the portal node.

Perhaps, we can try to make a dead branch elimination:

if (process.env.NODE_ENV === 'development') {
   (parent && parent.removeChild(this.node)) ||
       console.warn("Portal node has gone.");
} else {
   parent.removeChild(this.node);
}

What do you think? (Just trying to be safe.)

@diasbruno diasbruno changed the title Update Modal.js Hot reload removing portal node on storybook. Apr 19, 2019
@diasbruno
Copy link
Collaborator

Also, if you can provide more information about the setup, it might help...and thanks for reporting this issue.

@bertho-zero
Copy link
Author

bertho-zero commented Apr 23, 2019

This problem is not related to hot reload, I have the same problem in production as soon as the component is unmount.

I have a component that does not use document.body for the portal but uses a ref, like this:

class GnaGna extends Component {
  schedulerRef = React.createRef();

  getModalParent = () => this.schedulerRef.current;

  render() {
    return (
      <>
        <div ref={this.schedulerRef} />

        <ReactModal
          parentSelector={this.getModalParent}
        />
      </>
    );
  }
}

@bertho-zero
Copy link
Author

bertho-zero commented Apr 23, 2019

If I put the modal before I have an error with .appendChild, if I put it after the parent disappears before calling .removeChild.

@bertho-zero
Copy link
Author

I found a workaround:

class GnaGna extends Component {
  schedulerRef = React.createRef();

  modalRef = React.createRef();

  componentWillUnmount() {
    this.modalRef.current.node = null;
  }

  getModalParent = () => this.schedulerRef.current;

  render() {
    return (
      <>
        <div ref={this.schedulerRef} />

        <ReactModal
          ref={this.modalRef}
          parentSelector={this.getModalParent}
        />
      </>
    );
  }
}

@diasbruno
Copy link
Collaborator

This patch was superseded by #778.

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 this pull request may close these issues.

None yet

3 participants