[Fiber] Initial error boundaries #7993
Conversation
|
I think the best strategy would be to do the try catch right outside the while loop so that we don't go in and out of try statements. https://github.com/facebook/react/blob/master/src/renderers/shared/fiber/ReactFiberScheduler.js#L230 You'll know which one failed because it is the current value of "nextUnitOfWork". |
|
Can you rebase instead of merging from master to prevent the duplicate commits? |
|
I didn't want to rebase Seb's commits (they're still not in master). |
|
They are in master now |
|
Rebased. |
| // This flag gets set to true for error boundaries when they attempt to | ||
| // render the error path. This way we know that if rendering fails, we should | ||
| // skip this error boundary as failed and propagate the error above. | ||
| hasErrored: boolean, |
There was a problem hiding this comment.
I guess, the question is, what priority should the error have?
A) If no other priority can beat it, then we should just do this in a synchronous loop in place of performDeferredWork. If that's the case, then you can just have a separate try/catch around that one.
B) If another priority can render higher than the error set state, then this flag can be invalidated before we get around to rendering the error. Since we can still update the state of the tree before it errored.
So, I think A is probably the easiest option.
| handleErrorInBoundary(errorBoundary, error); | ||
| return; | ||
| } | ||
| // TODO: Do we need to reset nextUnitOfWork here? |
There was a problem hiding this comment.
It is always safe to set nextUnitOfWork to null because it will just find its place again.
|
|
||
| function handleErrorInBoundary(errorBoundary : Fiber, error : any) { | ||
| // The work below failed so we need to clear it out and try to render the error state. | ||
| // TODO: Do we need to clear all of these fields? How do we teardown an existing tree? |
There was a problem hiding this comment.
If we naively just clear everything here, we'll lose the ability to unmount them. I don't think we need to clear anything here.
We normally we reset everything to whatever the "current" tree represents on the way down the tree. The simplest way might be to restart from the root.
In theory the error boundary will already be a clone, it might be fine to just restart from it. However, unstable_handleError may be calling setState on a component above (or even a sibling) and that will start screwing everything up. If you restart from the root, that case will be covered.
|
Closing because I accidentally nuked my fork (and needs rebasing anyway). |
This builds on top of #7941 + new tests I added in #7949 (hence the master merge commit).
It adds basic support for error boundaries during mount to Fiber.
Commit for review: 833a7f8.
Test plan:
npm test -- ErrorB.Most of those 13 failing tests are failing because of the lack of
componentWillUpdateandcomponentWillReceiveProps.A few thoughts:
setStateincomponentWillMount.setState()are now caught and I didn't have to do anything special to make that works 😄 .