Adds support for static getDerivedStateFromError(error)
#67
Conversation
…cause `preact`'s support for `componentDidCatch()` does not pass this argument (preactjs/preact#819 (comment))
…olve preactjs#65. Includes 3 tests.
|
This current implementation is incorrect per this documentation: https://reactjs.org/docs/react-component.html#error-boundaries
Updating the code, starting with the tests. |
src/index.js
Outdated
| try { | ||
| if (nodeName.getDerivedStateFromProps) c.state = assign(assign({}, c.state), nodeName.getDerivedStateFromProps(c.props, c.state)); | ||
| else if (c.componentWillMount) c.componentWillMount(); | ||
| rendered = c.render(c.props, c.state, c.context); |
There was a problem hiding this comment.
You likely will want to wrap the recursive call to renderString in try/catch instead of the call to the component's render method so only errors of children will be caught.
There was a problem hiding this comment.
yea, this was a correct observation, it has been fixed
….org/docs/react-component.html#error-boundaries Merge branch 'componentDidCatch' into getDerivedStateFromError # Conflicts: # src/index.js
…method correctly
…Error` method correctly
…al components. This produces a much cleaner `try`/`catch` code around the error boundary. Unfortunately it changes the indent of the class based components code.
|
I commented on #66 but I think it's slightly different here - it seems like React has indicated |
|
i'd say this is not a good idea if react does not support it |
|
Closing as implemented in #305, upon the work done here. Thanks! |
This should resolve #65.
It includes code from #66. It is kept as a separate PR deliberately because
preactdoesn't currently have incoming support forstatic getDerivedStateFromError(error)preactjs/preact#819Documentation here:
https://reactjs.org/docs/react-component.html#static-getderivedstatefromerror