React: findDOMNode() throws inside Suspense

[will-hart]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

I have a component which listens for resize events (via a BlueprintJS ResizeSensor). When loading a component dynamically with lazy / Suspense, an exception occurs as the resize sensor appears to be unmounted:

Uncaught Error: Unable to find node on an unmounted component.
    at invariant (29.chunk.js:86295)
    at findCurrentFiberUsingSlowPath (29.chunk.js:90628)
    at findCurrentHostFiber (29.chunk.js:90640)
    at findHostInstanceWithWarning (29.chunk.js:106349)
    at findDOMNode (29.chunk.js:106869)
    at ResizeSensor.componentDidUpdate (29.chunk.js:10535)

The resize sensor should be removing listeners on unmount.

Demo: https://codesandbox.io/s/n4241q075l
Related: https://github.com/palantir/blueprint/issues/3141

What is the expected behavior?

No exception is thrown.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Tested with:

• React 16.6.1, fails
• React 16.6.0, works

[gaearon]

Can you test if it works on master?

[arianon]

Nope, it's broken on master, and after bisecting, I confirmed the regression was caused by #14083

EDIT: Something else I found is that before the regression, ResizeSensor.componentDidUpdate doesn't execute, but now it does.

EDIT 2: Yeah, ReactDOM.findDOMNode throws when invoked with a suspended component, here's how you can reproduce.

const React = require("react");
const ReactDOM = require("react-dom");
const ReactTestRenderer = require("react-test-renderer");
const { Suspense } = React;

let didSuspend = false;
function Suspend() {
  if (!didSuspend)
    throw {
      then() {
        didSuspend = true;
      }
    };
  return null;
}

class Component extends React.Component {
  componentDidUpdate() {
    ReactDOM.findDOMNode(this);
  }

  render() {
    return null;
  }
}

function App() {
  return (
    <Suspense fallback="Loading">
      <Component />
      <Suspend />
    </Suspense>
  );
}

const root = ReactTestRenderer.create(<App />);
root.update(<App />);

This is true both before and after the aforementioned PR, so in any case it looks like it helped us discover this :smile:

[gaearon]

EDIT: Something else I found is that before the regression, ResizeSensor.componentDidUpdate doesn't execute, but now it does.

This sounds like a separate problem, can you provide a test case for it?

[gaearon]

Here's a minimal repro with ReactDOM:

const Lazy = React.lazy(() => new Promise(resolve => resolve({
  default() {
    return 'hello';
  }
})))

class Child extends React.Component {
  componentDidUpdate() {
    ReactDOM.findDOMNode(this);
  }

  render() {
    return null;
  }
}

class App extends React.Component {
  state = {
    suspend: false
  };
  handleClick = () => {
    this.setState({ suspend: true });
  };
  render() {
    return (
      <React.Suspense fallback="Loading">
        <Child />
        <button onClick={this.handleClick}>Suspend</button>
        <Child />
        {this.state.suspend && <Lazy />}
      </React.Suspense>
    );
  }
}

ReactDOM.render(
  <App />,
  document.getElementById('container')
);

[gaearon]

Okay, so a part of this is covered by https://github.com/facebook/react/pull/14197. It got first broken by #14083 and got fixed in #14164.

However I think repro case in https://github.com/facebook/react/issues/14188#issuecomment-437705392 might be a separate issue. Looking into it.

[arianon]

This sounds like a separate problem, can you provide a test case for it?

Welp, turns out I'm wrong, I actually made a mistake when mocking componentDidUpdate which resulted in it not being called :stuck_out_tongue_closed_eyes:

[eps1lon]

Another reproducible example: https://codesandbox.io/s/zwzqp18ool

When does it throw:

• 16.6.0: never
• 16.6.1: on every "display lazy" click
• 16.7.0 until 16.8.6: on every "display lazy" click if the number of "force update" clicks is odd (meaning even number of render?)

[gaearon]

This was likely fixed in https://github.com/facebook/react/pull/15312. Can you verify on master?

[eps1lon]

This was likely fixed in #15312. Can you verify on master?

Yes, current master fixes https://github.com/facebook/react/issues/14188#issuecomment-481252546 for me.

[gaearon]

This will get into the next release then.

[zombieJ]

@gaearon @acdlite,
Seems it still happens. ~I tried to get mini reproduce but it seems happened in complex environment.~

User report reproduce: https://codesandbox.io/s/antd-reproduction-template-sbsiz
Quick click the nav item will throw with Unable to find node on an unmounted component.

ref: https://github.com/ant-design/ant-design/issues/17016

---

Update:

Create a mini reproduce about this: #15857

[gaearon]

Sorry, we didn’t release the fix yet.

[joshnabbott]

@gaearon any ETA on when this will be available?

[ambroselittle]

We are blocked by this, too (without a pretty significant workaround). Any sense of a release timeframe? @gaearon? Is there some way to find out (in general) when the "next" release is? Sorry if this is covered somewhere..

For the record, this is surfacing for us through Semantic UI React Dropdown. So we're beholden to them as well, but they point us here for the fix. :)

[wenisy]

Sorry, we didn’t release the fix yet.

@gaearon

any new updates?

[gaearon]

We've published 0.0.0-db3ae32b8 of all packages, which includes this fix. It is likely to be a release candidate for 16.9.0. You can give it a try now and see whether it resolves the issue for you. Thanks.

[gaearon]

(You should still ask packages to migrate away from findDOMNode though as it's eventually going to get deprecated.)

[MissLlq]

We've published 0.0.0-db3ae32b8 of all packages, which includes this fix. It is likely to be a release candidate for 16.9.0. You can give it a try now and see whether it resolves the issue for you. Thanks.

16.9.0 hadn't fix this bug!

[eps1lon]

We've published 0.0.0-db3ae32b8 of all packages, which includes this fix. It is likely to be a release candidate for 16.9.0. You can give it a try now and see whether it resolves the issue for you. Thanks.

16.9.0 hadn't fix this bug!

@MissLlq All reproductions linked in this issue (https://codesandbox.io/s/zwzqp18ool and https://codesandbox.io/s/n4241q075l) no longer crash with 16.9.

If you still encounter this error message with 16.9 please open a new issue.