React: Add error as argument to ReactDOM.render callback

[joeldenning]

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

What is the current behavior?
Currently, if the initial render for an application fails during ReactDOM.render(reactElement, domElement, callback), the callback will still be called, but without a way of knowing whether the render succeeded or not. See https://codepen.io/joeldenning/pen/mgLVLY?editors=0010.

This makes it tedious to know at the time that the initial render finishes whether the render was successful or not. Consider the tedious code required to reject a promise with an error if first render fails but resolve the promise with instance of component if it succeeds. As far as I can tell, this is the simplest possible code to achieve this.

new Promise((resolve, reject) => {
  let firstRenderErr = null
  try {
    const instanceOfComponent = ReactDOM.render(reactElement, domElement, () => {
      // setTimeout is necessary to give time for synchronous execution of
      // ReactDOM.render to finish so that the `instanceOfComponent` and 
      // `firstRenderErr` variables are defined when we call resolve
      setTimeout(() => {
        if (firstRenderErr) {
          reject(firstRenderErr)
        } else {
          resolve(instanceOfComponent)
        }
      })
    })
  } catch (err) {
    firstRenderErr = err
  }
})

What is the expected behavior?
I propose react starts calling the callback with two arguments, errback style:

ReactDOM.render(reactElement, domElement, (err, instanceOfComponent) => {
})

This change sets ReactDOM up for easy success/failure detection if ReactDOM.render() ever is performed asynchronously.

Additionally, it very much simplifies the tedious code from above:

new Promise((resolve, reject) => {
  ReactDOM.render(reactElement, domElement, (err, instanceOfComponent) => {
    if (err) {
      reject(err)
    } else {
      resolve(instanceOfComponent)
    }
  })
})

This would not be a breaking change, since adding arguments to the callback would not negatively affect any code (except situations where people are depending on the callback having zero arguments?? Seems unlikely).

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
All versions. I'm happy to attempt a PR for this if maintainers agree with the proposed change.

[ahtee]

You should be using error boundaries to catch errors. Virtual dom render method will render the mount or promise errors inside the error boundaries.
This should not be the job of the ReactDOM.render method to catch

[joeldenning]

@ahtee, I agree that using error boundaries is a good practice, but disagree that ReactDOM.render() should not report errors that occur.

Code that acts as a bridge between React code and other frontend code will call ReactDOM.render() and wait for it to finish by passing in a callback function. And I think it's pretty reasonable that that "bridge" code's callback function could expect to know whether the call to ReactDOM.render() succeeded or not. Placing the blame on the implementors of the React components does not change that the bridge code needs to know whether mounting the React components succeeded or failed.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[joeldenning]

Still would like to hear thoughts from maintainers on this.

[necolas]

We're not adding any new features to this API as it is being replaced in concurrent mode. You can also use an error boundary around the app