From 4b051f31f3e3f5efc8008d0b66480e063b2b861f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 2 Nov 2016 16:32:58 -0700 Subject: [PATCH 01/10] Refactor scheduling functions and introduce Task priority There was lots of duplication across all the scheduling functions. I think we're far enough along that we can start trying to clean some stuff up. Also introduces a new priority level provisionally called Task priority. This is for work that completes at the end of the current tick, after the current batch of work has been committed. It's different from Synchronous priority, which needs to complete immediately. A full implementation of Task priority will follow. It will replace the current batching solution. --- .../shared/fiber/ReactFiberScheduler.js | 233 ++++++++---------- .../shared/fiber/ReactPriorityLevel.js | 11 +- 2 files changed, 113 insertions(+), 131 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 205b1ad04a55..6e04191f54be 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -28,9 +28,12 @@ var { trapError, acknowledgeErrorInBoundary } = require('ReactFiberErrorBoundary var { NoWork, - LowPriority, - AnimationPriority, SynchronousPriority, + TaskPriority, + AnimationPriority, + HighPriority, + LowPriority, + OffscreenPriority, } = require('ReactPriorityLevel'); var { @@ -62,8 +65,8 @@ module.exports = function(config : HostConfig) { const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = ReactFiberCommitWork(config); - const scheduleAnimationCallback = config.scheduleAnimationCallback; - const scheduleDeferredCallback = config.scheduleDeferredCallback; + const hostScheduleAnimationCallback = config.scheduleAnimationCallback; + const hostScheduleDeferredCallback = config.scheduleDeferredCallback; const useSyncScheduling = config.useSyncScheduling; // The priority level to use when scheduling an update. @@ -86,6 +89,20 @@ module.exports = function(config : HostConfig) { let isAnimationCallbackScheduled : boolean = false; let isDeferredCallbackScheduled : boolean = false; + function scheduleAnimationCallback(callback) { + if (!isAnimationCallbackScheduled) { + isAnimationCallbackScheduled = true; + hostScheduleAnimationCallback(callback); + } + } + + function scheduleDeferredCallback(callback) { + if (!isDeferredCallbackScheduled) { + isDeferredCallbackScheduled = true; + hostScheduleDeferredCallback(callback); + } + } + function findNextUnitOfWork() { // Clear out roots with no more work on them. while (nextScheduledRoot && nextScheduledRoot.current.pendingWorkPriority === NoWork) { @@ -383,10 +400,7 @@ module.exports = function(config : HostConfig) { nextUnitOfWork = findNextUnitOfWork(); } } else { - if (!isDeferredCallbackScheduled) { - isDeferredCallbackScheduled = true; - scheduleDeferredCallback(performDeferredWork); - } + scheduleDeferredCallback(performDeferredWork); return; } } @@ -397,56 +411,19 @@ module.exports = function(config : HostConfig) { performAndHandleErrors(LowPriority, deadline); } - function scheduleDeferredWork(root : FiberRoot, priority : PriorityLevel) { - // We must reset the current unit of work pointer so that we restart the - // search from the root during the next tick, in case there is now higher - // priority work somewhere earlier than before. - if (priority <= nextPriorityLevel) { - nextUnitOfWork = null; - } - - // Set the priority on the root, without deprioritizing - if (root.current.pendingWorkPriority === NoWork || - priority <= root.current.pendingWorkPriority) { - root.current.pendingWorkPriority = priority; - } - - if (!root.isScheduled) { - root.isScheduled = true; - if (lastScheduledRoot) { - // Schedule ourselves to the end. - lastScheduledRoot.nextScheduledRoot = root; - lastScheduledRoot = root; - } else { - // We're the only work scheduled. - nextScheduledRoot = root; - lastScheduledRoot = root; - } - } - - if (!isDeferredCallbackScheduled) { - isDeferredCallbackScheduled = true; - scheduleDeferredCallback(performDeferredWork); - } - } - function performAnimationWorkUnsafe() { // Always start from the root nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && - nextPriorityLevel !== NoWork && - nextPriorityLevel <= AnimationPriority) { + nextPriorityLevel === AnimationPriority) { nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); if (!nextUnitOfWork) { // Keep searching for animation work until there's no more left nextUnitOfWork = findNextUnitOfWork(); } } - if (nextUnitOfWork && nextPriorityLevel > AnimationPriority) { - if (!isDeferredCallbackScheduled) { - isDeferredCallbackScheduled = true; - scheduleDeferredCallback(performDeferredWork); - } + if (nextUnitOfWork) { + scheduleCallbackAtPriority(nextPriorityLevel); } } @@ -455,32 +432,6 @@ module.exports = function(config : HostConfig) { performAndHandleErrors(AnimationPriority); } - function scheduleAnimationWork(root: FiberRoot, priorityLevel : PriorityLevel) { - // Set the priority on the root, without deprioritizing - if (root.current.pendingWorkPriority === NoWork || - priorityLevel <= root.current.pendingWorkPriority) { - root.current.pendingWorkPriority = priorityLevel; - } - - if (!root.isScheduled) { - root.isScheduled = true; - if (lastScheduledRoot) { - // Schedule ourselves to the end. - lastScheduledRoot.nextScheduledRoot = root; - lastScheduledRoot = root; - } else { - // We're the only work scheduled. - nextScheduledRoot = root; - lastScheduledRoot = root; - } - } - - if (!isAnimationCallbackScheduled) { - isAnimationCallbackScheduled = true; - scheduleAnimationCallback(performAnimationWork); - } - } - function scheduleErrorBoundaryWork(boundary : Fiber, priority) : FiberRoot { let root = null; let fiber = boundary; @@ -508,7 +459,6 @@ module.exports = function(config : HostConfig) { } function performSynchronousWorkUnsafe() { - // Perform work now nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === SynchronousPriority) { @@ -519,17 +469,7 @@ module.exports = function(config : HostConfig) { } } if (nextUnitOfWork) { - if (nextPriorityLevel > AnimationPriority) { - if (!isDeferredCallbackScheduled) { - isDeferredCallbackScheduled = true; - scheduleDeferredCallback(performDeferredWork); - } - return; - } - if (!isAnimationCallbackScheduled) { - isAnimationCallbackScheduled = true; - scheduleAnimationCallback(performAnimationWork); - } + scheduleCallbackAtPriority(nextPriorityLevel); } } @@ -544,45 +484,48 @@ module.exports = function(config : HostConfig) { } } - function scheduleSynchronousWork(root : FiberRoot) { - root.current.pendingWorkPriority = SynchronousPriority; - - if (root.isScheduled) { - // If we're already scheduled, we can bail out. - return; - } - root.isScheduled = true; - if (lastScheduledRoot) { - // Schedule ourselves to the end. - lastScheduledRoot.nextScheduledRoot = root; - lastScheduledRoot = root; - } else { - // We're the only work scheduled. - nextScheduledRoot = root; - lastScheduledRoot = root; + function performTaskWorkUnsafe() { + nextUnitOfWork = findNextUnitOfWork(); + while (nextUnitOfWork && + nextPriorityLevel === TaskPriority) { + nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); - if (!shouldBatchUpdates) { - // Unless in batched mode, perform work immediately - performSynchronousWork(); + if (!nextUnitOfWork) { + nextUnitOfWork = findNextUnitOfWork(); } } + if (nextUnitOfWork) { + scheduleCallbackAtPriority(nextPriorityLevel); + } } + // function performTaskWork() { + // performAndHandleErrors(TaskPriority); + // } + function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) { // The exact priority level doesn't matter, so long as it's in range of the // work (sync, animation, deferred) being performed. try { - if (priorityLevel === SynchronousPriority) { - performSynchronousWorkUnsafe(); - } else if (priorityLevel > AnimationPriority) { - if (!deadline) { - throw new Error('No deadline'); - } else { - performDeferredWorkUnsafe(deadline); - } - return; - } else { - performAnimationWorkUnsafe(); + switch (priorityLevel) { + case SynchronousPriority: + performSynchronousWorkUnsafe(); + return; + case TaskPriority: + performTaskWorkUnsafe(); + return; + case AnimationPriority: + performAnimationWorkUnsafe(); + return; + case HighPriority: + case LowPriority: + case OffscreenPriority: + if (!deadline) { + throw new Error('No deadline'); + } else { + performDeferredWorkUnsafe(deadline); + } + return; } } catch (error) { const failedUnitOfWork = nextUnitOfWork; @@ -672,15 +615,54 @@ module.exports = function(config : HostConfig) { } function scheduleWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) { - if (priorityLevel === NoWork) { - return; - } else if (priorityLevel === SynchronousPriority) { - scheduleSynchronousWork(root); - } else if (priorityLevel <= AnimationPriority) { - scheduleAnimationWork(root, priorityLevel); - } else { - scheduleDeferredWork(root, priorityLevel); - return; + // Set the priority on the root, without deprioritizing + if (root.current.pendingWorkPriority === NoWork || + priorityLevel <= root.current.pendingWorkPriority) { + root.current.pendingWorkPriority = priorityLevel; + } + + if (!root.isScheduled) { + root.isScheduled = true; + if (lastScheduledRoot) { + // Schedule ourselves to the end. + lastScheduledRoot.nextScheduledRoot = root; + lastScheduledRoot = root; + } else { + // We're the only work scheduled. + nextScheduledRoot = root; + lastScheduledRoot = root; + } + } + + // We must reset the current unit of work pointer so that we restart the + // search from the root during the next tick, in case there is now higher + // priority work somewhere earlier than before. + if (priorityLevel <= nextPriorityLevel) { + nextUnitOfWork = null; + } + + scheduleCallbackAtPriority(priorityLevel); + } + + function scheduleCallbackAtPriority(priorityLevel : PriorityLevel) { + switch (priorityLevel) { + case SynchronousPriority: + if (!shouldBatchUpdates) { + // Unless in batched mode, perform work immediately + performSynchronousWork(); + } + return; + case TaskPriority: + // Do nothing. Task work should be flushed after committing. + return; + case AnimationPriority: + scheduleAnimationCallback(performAnimationWork); + return; + case HighPriority: + case LowPriority: + case OffscreenPriority: + scheduleDeferredCallback(performDeferredWork); + return; } } @@ -746,7 +728,6 @@ module.exports = function(config : HostConfig) { return { scheduleWork: scheduleWork, - scheduleDeferredWork: scheduleDeferredWork, performWithPriority: performWithPriority, batchedUpdates: batchedUpdates, syncUpdates: syncUpdates, diff --git a/src/renderers/shared/fiber/ReactPriorityLevel.js b/src/renderers/shared/fiber/ReactPriorityLevel.js index b6930c2900a0..a0c9a7a7c07c 100644 --- a/src/renderers/shared/fiber/ReactPriorityLevel.js +++ b/src/renderers/shared/fiber/ReactPriorityLevel.js @@ -12,13 +12,14 @@ 'use strict'; -export type PriorityLevel = 0 | 1 | 2 | 3 | 4 | 5; +export type PriorityLevel = 0 | 1 | 2 | 3 | 4 | 5 | 6; module.exports = { NoWork: 0, // No work is pending. SynchronousPriority: 1, // For controlled text inputs. Synchronous side-effects. - AnimationPriority: 2, // Needs to complete before the next frame. - HighPriority: 3, // Interaction that needs to complete pretty soon to feel responsive. - LowPriority: 4, // Data fetching, or result from updating stores. - OffscreenPriority: 5, // Won't be visible but do the work in case it becomes visible. + TaskPriority: 2, // Completes at the end of the current tick. + AnimationPriority: 3, // Needs to complete before the next frame. + HighPriority: 4, // Interaction that needs to complete pretty soon to feel responsive. + LowPriority: 5, // Data fetching, or result from updating stores. + OffscreenPriority: 6, // Won't be visible but do the work in case it becomes visible. }; From 25d9db542c8137290ece77db2b3854360e3d0ec3 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 2 Nov 2016 22:32:23 -0700 Subject: [PATCH 02/10] Implement Task priority Task priority is similar to Synchronous priority. Both are flushed in the current tick. Synchronous priority is flushed immediately (e.g. sync work triggered by setState will flush before setState exits), where as Task is flushed after the current batch of work is committed. Currently used for batchedUpdates and nested sync updates. Task should also be used for componentDidUpdate/Mount and error boundary work. I'll add this in a later commit. --- scripts/fiber/tests-failing.txt | 6 +-- scripts/fiber/tests-passing.txt | 2 +- .../shared/fiber/ReactFiberScheduler.js | 37 ++++++++++++------- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 955d305f69e0..aa0193c847e4 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -554,9 +554,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentNestedState-test.js * should provide up to date values for props -src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js -* should support setting state - src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js * should still throw when rendering to undefined * should be able to switch between rendering null and a normal tag @@ -565,6 +562,9 @@ src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js * throws when rendering null at the top level * preserves the dom node during updates +src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js +* lets different boundaries catch their own first errors + src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js * should warn for duplicated keys with component stack info diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 42e9b95c1dfb..c93ed2b54912 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1016,6 +1016,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentDOMMinima * should not render extra nodes for non-interpolated text src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponentState-test.js +* should support setting state * should batch unmounts src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js @@ -1058,7 +1059,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js * catches errors in componentDidMount * catches errors in componentDidUpdate * propagates errors inside boundary during componentDidMount -* lets different boundaries catch their own first errors src/renderers/shared/stack/reconciler/__tests__/ReactIdentity-test.js * should allow key property to express identity diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 6e04191f54be..93564cd5b252 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -167,7 +167,7 @@ module.exports = function(config : HostConfig) { commitInsertion(effectfulFiber); // Clear the "placement" from effect tag so that we know that this is inserted, before // any life-cycles like componentDidMount gets called. - effectfulFiber.effectTag = NoEffect; + effectfulFiber.effectTag ^= Placement; break; } case PlacementAndUpdate: @@ -176,7 +176,7 @@ module.exports = function(config : HostConfig) { commitInsertion(effectfulFiber); // Clear the "placement" from effect tag so that we know that this is inserted, before // any life-cycles like componentDidMount gets called. - effectfulFiber.effectTag = Update; + effectfulFiber.effectTag ^= Placement; // Update const current = effectfulFiber.alternate; @@ -250,6 +250,7 @@ module.exports = function(config : HostConfig) { if (allTrappedErrors) { handleErrors(allTrappedErrors); } + performTaskWork(); } function resetWorkPriority(workInProgress : Fiber) { @@ -499,9 +500,9 @@ module.exports = function(config : HostConfig) { } } - // function performTaskWork() { - // performAndHandleErrors(TaskPriority); - // } + function performTaskWork() { + performAndHandleErrors(TaskPriority); + } function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) { // The exact priority level doesn't matter, so long as it's in range of the @@ -611,7 +612,14 @@ module.exports = function(config : HostConfig) { } function scheduleWork(root : FiberRoot) { - scheduleWorkAtPriority(root, priorityContext); + let priorityLevel = priorityContext; + + // If we're in a batch, switch to task priority + if (priorityLevel === SynchronousPriority && shouldBatchUpdates) { + priorityLevel = TaskPriority; + } + + scheduleWorkAtPriority(root, priorityLevel); } function scheduleWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) { @@ -647,10 +655,8 @@ module.exports = function(config : HostConfig) { function scheduleCallbackAtPriority(priorityLevel : PriorityLevel) { switch (priorityLevel) { case SynchronousPriority: - if (!shouldBatchUpdates) { - // Unless in batched mode, perform work immediately - performSynchronousWork(); - } + // Perform work immediately + performSynchronousWork(); return; case TaskPriority: // Do nothing. Task work should be flushed after committing. @@ -667,7 +673,12 @@ module.exports = function(config : HostConfig) { } function scheduleUpdate(fiber : Fiber) { - const priorityLevel = priorityContext; + let priorityLevel = priorityContext; + // If we're in a batch, switch to task priority + if (priorityLevel === SynchronousPriority && shouldBatchUpdates) { + priorityLevel = TaskPriority; + } + while (true) { if (fiber.pendingWorkPriority === NoWork || fiber.pendingWorkPriority >= priorityLevel) { @@ -709,9 +720,9 @@ module.exports = function(config : HostConfig) { return fn(); } finally { shouldBatchUpdates = prev; - // If we've exited the batch, perform any scheduled sync work + // If we've exited the batch, perform any scheduled task work if (!shouldBatchUpdates) { - performSynchronousWork(); + performTaskWork(); } } } From 2cc85e6657f03f7707a0fc7392f039f94fbce917 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 3 Nov 2016 15:24:31 -0700 Subject: [PATCH 03/10] Make error boundaries use Task Priority I have all but one error fixed. Not sure how tricky the last one is, but I'm cautiously optimistic. Pushing to show my current progress. --- scripts/fiber/tests-failing.txt | 5 +++- scripts/fiber/tests-passing.txt | 3 +-- .../shared/fiber/ReactFiberScheduler.js | 27 ++++++++++--------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index aa0193c847e4..c1447b2b02b8 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate +src/renderers/dom/__tests__/ReactDOMProduction-test.js +* should throw with an error code in production + src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names @@ -563,7 +566,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js * preserves the dom node during updates src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js -* lets different boundaries catch their own first errors +* propagates errors on retry on mounting src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js * should warn for duplicated keys with component stack info diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c93ed2b54912..3f0d6bda18b6 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods -* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children @@ -1034,7 +1033,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js * renders an error state if child throws in constructor * renders an error state if child throws in componentWillMount * mounts the error message if mounting fails -* propagates errors on retry on mounting * propagates errors inside boundary during componentWillMount * propagates errors inside boundary while rendering error state * does not register event handlers for unmounted children @@ -1059,6 +1057,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js * catches errors in componentDidMount * catches errors in componentDidUpdate * propagates errors inside boundary during componentDidMount +* lets different boundaries catch their own first errors src/renderers/shared/stack/reconciler/__tests__/ReactIdentity-test.js * should allow key property to express identity diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 93564cd5b252..36dba6309529 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -485,11 +485,12 @@ module.exports = function(config : HostConfig) { } } - function performTaskWorkUnsafe() { + function performTaskWorkUnsafe(ignoreUnmountingErrors : boolean) { nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === TaskPriority) { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); + nextUnitOfWork = + performUnitOfWork(nextUnitOfWork, ignoreUnmountingErrors); if (!nextUnitOfWork) { nextUnitOfWork = findNextUnitOfWork(); @@ -513,7 +514,7 @@ module.exports = function(config : HostConfig) { performSynchronousWorkUnsafe(); return; case TaskPriority: - performTaskWorkUnsafe(); + performTaskWorkUnsafe(false); return; case AnimationPriority: performAnimationWorkUnsafe(); @@ -527,6 +528,8 @@ module.exports = function(config : HostConfig) { performDeferredWorkUnsafe(deadline); } return; + default: + return; } } catch (error) { const failedUnitOfWork = nextUnitOfWork; @@ -546,8 +549,12 @@ module.exports = function(config : HostConfig) { let nextTrappedErrors = initialTrappedErrors; let firstUncaughtError = null; + const previousPriorityContext = priorityContext; + priorityContext = TaskPriority; + // In each phase, we will attempt to pass errors to boundaries and re-render them. // If we get more errors, we propagate them to higher boundaries in the next iterations. + while (nextTrappedErrors) { const trappedErrors = nextTrappedErrors; nextTrappedErrors = null; @@ -578,16 +585,10 @@ module.exports = function(config : HostConfig) { // We will process an update caused by each error boundary synchronously. affectedBoundaries.forEach(boundary => { - const priority = priorityContext; - const root = scheduleErrorBoundaryWork(boundary, priority); - // This should use findNextUnitOfWork() when synchronous scheduling is implemented. - let fiber = cloneFiber(root.current, priority); + scheduleUpdate(boundary); try { - while (fiber) { - // TODO: this is the only place where we recurse and it's unfortunate. - // (This may potentially get us into handleErrors() again.) - fiber = performUnitOfWork(fiber, true); - } + nextUnitOfWork = findNextUnitOfWork(); + performTaskWorkUnsafe(true); } catch (nextError) { // If it throws, propagate the error. nextTrappedErrors = nextTrappedErrors || []; @@ -598,6 +599,8 @@ module.exports = function(config : HostConfig) { ReactCurrentOwner.current = null; + priorityContext = previousPriorityContext; + // Surface the first error uncaught by the boundaries to the user. if (firstUncaughtError) { // We need to make sure any future root can get scheduled despite these errors. From b407b9a223d180c8d46f0e767c53247f0c35d670 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 3 Nov 2016 22:42:50 -0700 Subject: [PATCH 04/10] Remove recursion from handleErrors Changed the algorithm of handleErrors a bit to ensure that boundaries are not revisited once they are acknowledged. --- scripts/fiber/tests-failing.txt | 6 - scripts/fiber/tests-passing.txt | 2 + .../shared/fiber/ReactFiberCommitWork.js | 8 +- .../shared/fiber/ReactFiberErrorBoundary.js | 53 ----- .../shared/fiber/ReactFiberScheduler.js | 197 +++++++++++------- 5 files changed, 130 insertions(+), 136 deletions(-) delete mode 100644 src/renderers/shared/fiber/ReactFiberErrorBoundary.js diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index c1447b2b02b8..c78722ebb6f1 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,9 +87,6 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate -src/renderers/dom/__tests__/ReactDOMProduction-test.js -* should throw with an error code in production - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names @@ -565,9 +562,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactEmptyComponent-test.js * throws when rendering null at the top level * preserves the dom node during updates -src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js -* propagates errors on retry on mounting - src/renderers/shared/stack/reconciler/__tests__/ReactMultiChild-test.js * should warn for duplicated keys with component stack info diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 3f0d6bda18b6..e105c31f5c4a 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,6 +438,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children @@ -1033,6 +1034,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactErrorBoundaries-test.js * renders an error state if child throws in constructor * renders an error state if child throws in componentWillMount * mounts the error message if mounting fails +* propagates errors on retry on mounting * propagates errors inside boundary during componentWillMount * propagates errors inside boundary while rendering error state * does not register event handlers for unmounted children diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index b23e5bdd6f8d..10ef91b8e449 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -12,7 +12,7 @@ 'use strict'; -import type { TrappedError } from 'ReactFiberErrorBoundary'; +import type { TrappedError } from 'ReactFiberScheduler'; import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; @@ -24,7 +24,6 @@ var { HostComponent, HostText, } = ReactTypeOfWork; -var { trapError } = require('ReactFiberErrorBoundary'); var { callCallbacks } = require('ReactFiberUpdateQueue'); var { @@ -33,7 +32,10 @@ var { Callback, } = require('ReactTypeOfSideEffect'); -module.exports = function(config : HostConfig) { +module.exports = function( + config : HostConfig, + trapError : (boundary : Fiber, error: Error) => TrappedError +) { const updateContainer = config.updateContainer; const commitUpdate = config.commitUpdate; diff --git a/src/renderers/shared/fiber/ReactFiberErrorBoundary.js b/src/renderers/shared/fiber/ReactFiberErrorBoundary.js deleted file mode 100644 index 726888215596..000000000000 --- a/src/renderers/shared/fiber/ReactFiberErrorBoundary.js +++ /dev/null @@ -1,53 +0,0 @@ -/** - * Copyright 2013-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - * @providesModule ReactFiberErrorBoundary - * @flow - */ - -'use strict'; - -import type { Fiber } from 'ReactFiber'; - -var { - ClassComponent, -} = require('ReactTypeOfWork'); - -export type TrappedError = { - boundary: Fiber | null, - error: any, -}; - -function findClosestErrorBoundary(fiber : Fiber): Fiber | null { - let maybeErrorBoundary = fiber.return; - while (maybeErrorBoundary) { - if (maybeErrorBoundary.tag === ClassComponent) { - const instance = maybeErrorBoundary.stateNode; - if (typeof instance.unstable_handleError === 'function') { - return maybeErrorBoundary; - } - } - maybeErrorBoundary = maybeErrorBoundary.return; - } - return null; -} - -function trapError(fiber : Fiber, error : any) : TrappedError { - return { - boundary: findClosestErrorBoundary(fiber), - error, - }; -} - -function acknowledgeErrorInBoundary(boundary : Fiber, error : any) { - const instance = boundary.stateNode; - instance.unstable_handleError(error); -} - -exports.trapError = trapError; -exports.acknowledgeErrorInBoundary = acknowledgeErrorInBoundary; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 36dba6309529..3164e3e3888c 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -12,7 +12,6 @@ 'use strict'; -import type { TrappedError } from 'ReactFiberErrorBoundary'; import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig, Deadline } from 'ReactFiberReconciler'; @@ -24,7 +23,6 @@ var ReactFiberCommitWork = require('ReactFiberCommitWork'); var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); -var { trapError, acknowledgeErrorInBoundary } = require('ReactFiberErrorBoundary'); var { NoWork, @@ -51,6 +49,7 @@ var { var { HostContainer, + ClassComponent, } = require('ReactTypeOfWork'); if (__DEV__) { @@ -59,11 +58,16 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; +export type TrappedError = { + boundary: Fiber | null, + error: any, +}; + module.exports = function(config : HostConfig) { const { beginWork } = ReactFiberBeginWork(config, scheduleUpdate); const { completeWork } = ReactFiberCompleteWork(config); const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } = - ReactFiberCommitWork(config); + ReactFiberCommitWork(config, trapError); const hostScheduleAnimationCallback = config.scheduleAnimationCallback; const hostScheduleDeferredCallback = config.scheduleDeferredCallback; @@ -89,6 +93,11 @@ module.exports = function(config : HostConfig) { let isAnimationCallbackScheduled : boolean = false; let isDeferredCallbackScheduled : boolean = false; + let nextTrappedErrors : Array | null = null; + let isHandlingErrors : boolean = false; + // Boundaries that have been acknowledged + let knownBoundaries : Set = new Set(); + function scheduleAnimationCallback(callback) { if (!isAnimationCallbackScheduled) { isAnimationCallbackScheduled = true; @@ -246,10 +255,14 @@ module.exports = function(config : HostConfig) { } } - // Now that the tree has been committed, we can handle errors. if (allTrappedErrors) { - handleErrors(allTrappedErrors); + if (nextTrappedErrors) { + nextTrappedErrors.push.apply(nextTrappedErrors, allTrappedErrors); + } else { + nextTrappedErrors = allTrappedErrors; + } } + performTaskWork(); } @@ -433,32 +446,6 @@ module.exports = function(config : HostConfig) { performAndHandleErrors(AnimationPriority); } - function scheduleErrorBoundaryWork(boundary : Fiber, priority) : FiberRoot { - let root = null; - let fiber = boundary; - while (fiber) { - fiber.pendingWorkPriority = priority; - if (fiber.alternate) { - fiber.alternate.pendingWorkPriority = priority; - } - if (!fiber.return) { - if (fiber.tag === HostContainer) { - // We found the root. - // Remember it so we can update it. - root = ((fiber.stateNode : any) : FiberRoot); - break; - } else { - throw new Error('Invalid root'); - } - } - fiber = fiber.return; - } - if (!root) { - throw new Error('Could not find root from the boundary.'); - } - return root; - } - function performSynchronousWorkUnsafe() { nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && @@ -512,13 +499,13 @@ module.exports = function(config : HostConfig) { switch (priorityLevel) { case SynchronousPriority: performSynchronousWorkUnsafe(); - return; + break; case TaskPriority: performTaskWorkUnsafe(false); - return; + break; case AnimationPriority: performAnimationWorkUnsafe(); - return; + break; case HighPriority: case LowPriority: case OffscreenPriority: @@ -527,12 +514,16 @@ module.exports = function(config : HostConfig) { } else { performDeferredWorkUnsafe(deadline); } - return; + break; default: - return; + break; } } catch (error) { const failedUnitOfWork = nextUnitOfWork; + + // Clean-up + ReactCurrentOwner.current = null; + // Reset because it points to the error boundary: nextUnitOfWork = null; if (!failedUnitOfWork) { @@ -541,79 +532,137 @@ module.exports = function(config : HostConfig) { throw error; } const trappedError = trapError(failedUnitOfWork, error); - handleErrors([trappedError]); + if (nextTrappedErrors) { + nextTrappedErrors.push(trappedError); + } else { + nextTrappedErrors = [trappedError]; + } + } + + // If there were any errors, handle them now + if (nextTrappedErrors) { + handleErrors(); } } - function handleErrors(initialTrappedErrors : Array) : void { - let nextTrappedErrors = initialTrappedErrors; + function handleErrors() : void { + // Prevent recursion + if (isHandlingErrors) { + return; + } + isHandlingErrors = true; + let firstUncaughtError = null; + // All work created by error boundaries should have Task priority + // so that it finishes before this function exits const previousPriorityContext = priorityContext; priorityContext = TaskPriority; - // In each phase, we will attempt to pass errors to boundaries and re-render them. - // If we get more errors, we propagate them to higher boundaries in the next iterations. - - while (nextTrappedErrors) { - const trappedErrors = nextTrappedErrors; - nextTrappedErrors = null; - - // Pass errors to all affected boundaries. - const affectedBoundaries : Set = new Set(); - trappedErrors.forEach(trappedError => { + // Keep looping until there are no more trapped errors + while (true) { + while (nextTrappedErrors && nextTrappedErrors.length) { + const trappedError = nextTrappedErrors.shift(); const boundary = trappedError.boundary; const error = trappedError.error; if (!boundary) { firstUncaughtError = firstUncaughtError || error; - return; + continue; } // Don't visit boundaries twice. - if (affectedBoundaries.has(boundary)) { - return; + if (knownBoundaries.has(boundary)) { + continue; } - // Give error boundary a chance to update its state. try { + knownBoundaries.add(boundary); + // Give error boundary a chance to update its state. + // Updates will be scheduled with Task priority. acknowledgeErrorInBoundary(boundary, error); - affectedBoundaries.add(boundary); } catch (nextError) { - // If it throws, propagate the error. - nextTrappedErrors = nextTrappedErrors || []; - nextTrappedErrors.push(trapError(boundary, nextError)); + // If an error is thrown, propagate the error to the next boundary + const te = trapError(boundary, nextError); + if (te.boundary) { + // If a boundary is found, push trapped error onto array + nextTrappedErrors.push(te); + } else { + // Otherwise, we'll need to throw + firstUncaughtError = firstUncaughtError || te.error; + } } - }); + } - // We will process an update caused by each error boundary synchronously. - affectedBoundaries.forEach(boundary => { - scheduleUpdate(boundary); - try { - nextUnitOfWork = findNextUnitOfWork(); - performTaskWorkUnsafe(true); - } catch (nextError) { - // If it throws, propagate the error. - nextTrappedErrors = nextTrappedErrors || []; - nextTrappedErrors.push(trapError(boundary, nextError)); + + // Now that we attempt to flush any work that was scheduled by the boundaries + // If this creates errors, they will be pushed to nextTrappedErrors and the loop will continue + try { + performTaskWorkUnsafe(true); + } catch (error) { + const failedUnitOfWork = nextUnitOfWork; + // Reset because it points to the error boundary: + nextUnitOfWork = null; + if (!failedUnitOfWork) { + // We shouldn't end up here because nextUnitOfWork + // should always be set while work is being performed. + throw error; } - }); - } + const trappedError = trapError(failedUnitOfWork, error); + if (nextTrappedErrors) { + nextTrappedErrors.push(trappedError); + } else { + nextTrappedErrors = [trappedError]; + } + } - ReactCurrentOwner.current = null; + if (!nextTrappedErrors || + !nextTrappedErrors.length || + firstUncaughtError) { + break; + } + } + + nextTrappedErrors = null; + knownBoundaries.clear(); priorityContext = previousPriorityContext; + isHandlingErrors = false; - // Surface the first error uncaught by the boundaries to the user. if (firstUncaughtError) { // We need to make sure any future root can get scheduled despite these errors. // Currently after throwing, nothing gets scheduled because these fields are set. // FIXME: this is likely a wrong fix! It's still better than ignoring updates though. nextScheduledRoot = null; lastScheduledRoot = null; - - // Throw any unhandled errors. throw firstUncaughtError; } } + function findClosestErrorBoundary(fiber : Fiber): Fiber | null { + let maybeErrorBoundary = fiber.return; + while (maybeErrorBoundary) { + if (maybeErrorBoundary.tag === ClassComponent) { + const instance = maybeErrorBoundary.stateNode; + if (typeof instance.unstable_handleError === 'function' && + !knownBoundaries.has(maybeErrorBoundary)) { + return maybeErrorBoundary; + } + } + maybeErrorBoundary = maybeErrorBoundary.return; + } + return null; + } + + function trapError(fiber : Fiber, error : any) : TrappedError { + return { + boundary: findClosestErrorBoundary(fiber), + error, + }; + } + + function acknowledgeErrorInBoundary(boundary : Fiber, error : any) { + const instance = boundary.stateNode; + instance.unstable_handleError(error); + } + function scheduleWork(root : FiberRoot) { let priorityLevel = priorityContext; From 0f5d44c1ce59c5aba30650125dba3851bb422d35 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 4 Nov 2016 10:32:33 -0700 Subject: [PATCH 05/10] Add incremental error boundary test Discovered an edge case: a noop error boundary will break in incremental mode unless you explicitly schedule an update. --- scripts/fiber/tests-failing.txt | 3 +++ scripts/fiber/tests-passing.txt | 2 +- .../shared/fiber/ReactFiberScheduler.js | 5 ++++ .../fiber/__tests__/ReactIncremental-test.js | 23 +++++++++++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index c78722ebb6f1..67bbbea71ec0 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate +src/renderers/dom/__tests__/ReactDOMProduction-test.js +* should throw with an error code in production + src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index e105c31f5c4a..b42a892af2cc 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods -* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children @@ -791,6 +790,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * skips will/DidUpdate when bailing unless an update was already in progress * performs batched updates at the end of the batch * can nest batchedUpdates +* propagates an error from a noop error boundary src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js * handles isMounted even when the initial render is deferred diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 3164e3e3888c..dafb2652fba8 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -575,9 +575,14 @@ module.exports = function(config : HostConfig) { } try { knownBoundaries.add(boundary); + // Give error boundary a chance to update its state. // Updates will be scheduled with Task priority. acknowledgeErrorInBoundary(boundary, error); + + // Schedule an update, in case the boundary didn't call setState + // on itself + scheduleUpdate(boundary); } catch (nextError) { // If an error is thrown, propagate the error to the next boundary const te = trapError(boundary, nextError); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 5df5c10caf80..8fa9afe489d0 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1465,4 +1465,27 @@ describe('ReactIncremental', () => { ]); expect(instance.state.n).toEqual(4); }); + + it('propagates an error from a noop error boundary', () => { + class NoopBoundary extends React.Component { + unstable_handleError() { + // Noop + } + render() { + return this.props.children; + } + } + + function RenderError() { + throw new Error('render error'); + } + + ReactNoop.render( + + + + ); + + expect(ReactNoop.flush).toThrow('render error'); + }); }); From cd104022a90ea5a1f775020fa62c35c13ea3a344 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Nov 2016 18:16:16 +0000 Subject: [PATCH 06/10] Refactor error boundaries in Fiber --- scripts/fiber/tests-failing.txt | 3 - scripts/fiber/tests-passing.txt | 1 + .../shared/fiber/ReactFiberCommitWork.js | 72 +++------ .../shared/fiber/ReactFiberScheduler.js | 153 +++++++----------- 4 files changed, 81 insertions(+), 148 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 67bbbea71ec0..c78722ebb6f1 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,9 +87,6 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate -src/renderers/dom/__tests__/ReactDOMProduction-test.js -* should throw with an error code in production - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index b42a892af2cc..71f078f40969 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,6 +438,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js index 10ef91b8e449..7a902e20977b 100644 --- a/src/renderers/shared/fiber/ReactFiberCommitWork.js +++ b/src/renderers/shared/fiber/ReactFiberCommitWork.js @@ -12,7 +12,6 @@ 'use strict'; -import type { TrappedError } from 'ReactFiberScheduler'; import type { Fiber } from 'ReactFiber'; import type { FiberRoot } from 'ReactFiberRoot'; import type { HostConfig } from 'ReactFiberReconciler'; @@ -34,7 +33,7 @@ var { module.exports = function( config : HostConfig, - trapError : (boundary : Fiber, error: Error) => TrappedError + trapError : (fiber : Fiber, error: Error, isUnmounting : boolean) => void ) { const updateContainer = config.updateContainer; @@ -158,10 +157,7 @@ module.exports = function( } } - function commitNestedUnmounts(root : Fiber): Array | null { - // Since errors are rare, we allocate this array on demand. - let trappedErrors = null; - + function commitNestedUnmounts(root : Fiber): void { // While we're inside a removed host node we don't want to call // removeChild on the inner nodes because they're removed by the top // call anyway. We also want to call componentWillUnmount on all @@ -169,58 +165,39 @@ module.exports = function( // we do an inner loop while we're still inside the host node. let node : Fiber = root; while (true) { - const error = commitUnmount(node); - if (error) { - trappedErrors = trappedErrors || []; - trappedErrors.push(error); - } + commitUnmount(node); if (node.child) { // TODO: Coroutines need to visit the stateNode. node = node.child; continue; } if (node === root) { - return trappedErrors; + return; } while (!node.sibling) { if (!node.return || node.return === root) { - return trappedErrors; + return; } node = node.return; } node = node.sibling; } - return trappedErrors; } - function unmountHostComponents(parent, current): Array | null { - // Since errors are rare, we allocate this array on demand. - let trappedErrors = null; - + function unmountHostComponents(parent, current): void { // We only have the top Fiber that was inserted but we need recurse down its // children to find all the terminal nodes. let node : Fiber = current; while (true) { if (node.tag === HostComponent || node.tag === HostText) { - const errors = commitNestedUnmounts(node); - if (errors) { - if (!trappedErrors) { - trappedErrors = errors; - } else { - trappedErrors.push.apply(trappedErrors, errors); - } - } + commitNestedUnmounts(node); // After all the children have unmounted, it is now safe to remove the // node from the tree. if (parent) { removeChild(parent, node.stateNode); } } else { - const error = commitUnmount(node); - if (error) { - trappedErrors = trappedErrors || []; - trappedErrors.push(error); - } + commitUnmount(node); if (node.child) { // TODO: Coroutines need to visit the stateNode. node = node.child; @@ -228,24 +205,23 @@ module.exports = function( } } if (node === current) { - return trappedErrors; + return; } while (!node.sibling) { if (!node.return || node.return === current) { - return trappedErrors; + return; } node = node.return; } node = node.sibling; } - return trappedErrors; } - function commitDeletion(current : Fiber) : Array | null { + function commitDeletion(current : Fiber) : void { // Recursively delete all host nodes from the parent. const parent = getHostParent(current); // Detach refs and call componentWillUnmount() on the whole subtree. - const trappedErrors = unmountHostComponents(parent, current); + unmountHostComponents(parent, current); // Cut off the return pointers to disconnect it from the tree. Ideally, we // should clear the child pointer of the parent alternate to let this @@ -258,11 +234,9 @@ module.exports = function( current.alternate.child = null; current.alternate.return = null; } - - return trappedErrors; } - function commitUnmount(current : Fiber) : TrappedError | null { + function commitUnmount(current : Fiber) : void { switch (current.tag) { case ClassComponent: { detachRef(current); @@ -270,17 +244,14 @@ module.exports = function( if (typeof instance.componentWillUnmount === 'function') { const error = tryCallComponentWillUnmount(instance); if (error) { - return trapError(current, error); + trapError(current, error, true); } } - return null; + return; } case HostComponent: { detachRef(current); - return null; - } - default: { - return null; + return; } } } @@ -325,7 +296,7 @@ module.exports = function( } } - function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : TrappedError | null { + function commitLifeCycles(current : ?Fiber, finishedWork : Fiber) : void { switch (finishedWork.tag) { case ClassComponent: { const instance = finishedWork.stateNode; @@ -355,9 +326,9 @@ module.exports = function( } } if (error) { - return trapError(finishedWork, error); + trapError(finishedWork, error, false); } - return null; + return; } case HostContainer: { const rootFiber = finishedWork.stateNode; @@ -366,15 +337,16 @@ module.exports = function( rootFiber.callbackList = null; callCallbacks(callbackList, rootFiber.current.child.stateNode); } + return; } case HostComponent: { const instance : I = finishedWork.stateNode; attachRef(current, finishedWork, instance); - return null; + return; } case HostText: { // We have no life-cycles associated with text. - return null; + return; } default: throw new Error('This unit of work tag should not have side-effects.'); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index dafb2652fba8..f8f97848892a 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -58,7 +58,7 @@ if (__DEV__) { var timeHeuristicForUnitOfWork = 1; -export type TrappedError = { +type TrappedError = { boundary: Fiber | null, error: any, }; @@ -93,10 +93,9 @@ module.exports = function(config : HostConfig) { let isAnimationCallbackScheduled : boolean = false; let isDeferredCallbackScheduled : boolean = false; + // Caught errors and error boundaries that are currently handling them + let activeErrorBoundaries : Set | null = null; let nextTrappedErrors : Array | null = null; - let isHandlingErrors : boolean = false; - // Boundaries that have been acknowledged - let knownBoundaries : Set = new Set(); function scheduleAnimationCallback(callback) { if (!isAnimationCallbackScheduled) { @@ -159,13 +158,6 @@ module.exports = function(config : HostConfig) { function commitAllWork(finishedWork : Fiber, ignoreUnmountingErrors : boolean) { // Commit all the side-effects within a tree. - - // Commit phase is meant to be atomic and non-interruptible. - // Any errors raised in it should be handled after it is over - // so that we don't end up in an inconsistent state due to user code. - // We'll keep track of all caught errors and handle them later. - let allTrappedErrors = null; - // First, we'll perform all the host insertions, updates, deletions and // ref unmounts. let effectfulFiber = finishedWork.firstEffect; @@ -201,18 +193,11 @@ module.exports = function(config : HostConfig) { case DeletionAndCallback: // Deletion might cause an error in componentWillUnmount(). // We will continue nevertheless and handle those later on. - const trappedErrors = commitDeletion(effectfulFiber); - // There is a special case where we completely ignore errors. + // Note: There is a special case where we completely ignore errors. // It happens when we already caught an error earlier, and the update // is caused by an error boundary trying to render an error message. // In this case, we want to blow away the tree without catching errors. - if (trappedErrors && !ignoreUnmountingErrors) { - if (!allTrappedErrors) { - allTrappedErrors = trappedErrors; - } else { - allTrappedErrors.push.apply(allTrappedErrors, trappedErrors); - } - } + commitDeletion(effectfulFiber, ignoreUnmountingErrors); break; } @@ -226,11 +211,7 @@ module.exports = function(config : HostConfig) { while (effectfulFiber) { if (effectfulFiber.effectTag & (Update | Callback)) { const current = effectfulFiber.alternate; - const trappedError = commitLifeCycles(current, effectfulFiber); - if (trappedError) { - allTrappedErrors = allTrappedErrors || []; - allTrappedErrors.push(trappedError); - } + commitLifeCycles(current, effectfulFiber); } const next = effectfulFiber.nextEffect; // Ensure that we clean these up so that we don't accidentally keep them. @@ -248,19 +229,7 @@ module.exports = function(config : HostConfig) { if (finishedWork.effectTag !== NoEffect) { const current = finishedWork.alternate; commitWork(current, finishedWork); - const trappedError = commitLifeCycles(current, finishedWork); - if (trappedError) { - allTrappedErrors = allTrappedErrors || []; - allTrappedErrors.push(trappedError); - } - } - - if (allTrappedErrors) { - if (nextTrappedErrors) { - nextTrappedErrors.push.apply(nextTrappedErrors, allTrappedErrors); - } else { - nextTrappedErrors = allTrappedErrors; - } + commitLifeCycles(current, finishedWork); } performTaskWork(); @@ -519,11 +488,10 @@ module.exports = function(config : HostConfig) { break; } } catch (error) { - const failedUnitOfWork = nextUnitOfWork; - // Clean-up ReactCurrentOwner.current = null; + const failedUnitOfWork = nextUnitOfWork; // Reset because it points to the error boundary: nextUnitOfWork = null; if (!failedUnitOfWork) { @@ -531,12 +499,7 @@ module.exports = function(config : HostConfig) { // should always be set while work is being performed. throw error; } - const trappedError = trapError(failedUnitOfWork, error); - if (nextTrappedErrors) { - nextTrappedErrors.push(trappedError); - } else { - nextTrappedErrors = [trappedError]; - } + trapError(failedUnitOfWork, error, false); } // If there were any errors, handle them now @@ -546,21 +509,27 @@ module.exports = function(config : HostConfig) { } function handleErrors() : void { - // Prevent recursion - if (isHandlingErrors) { + // Prevent recursion. + // TODO: remove the codepath that causes this recursion in the first place. + if (activeErrorBoundaries) { return; } - isHandlingErrors = true; + // Start tracking active boundaries. + activeErrorBoundaries = new Set(); + + // If we find unhandled errors, we'll only rethrow the first one. let firstUncaughtError = null; // All work created by error boundaries should have Task priority - // so that it finishes before this function exits + // so that it finishes before this function exits. const previousPriorityContext = priorityContext; priorityContext = TaskPriority; - // Keep looping until there are no more trapped errors - while (true) { + // Keep looping until there are no more trapped errors, or until we find + // an unhandled error. + while (nextTrappedErrors && nextTrappedErrors.length && !firstUncaughtError) { + // First, find all error boundaries and notify them about errors. while (nextTrappedErrors && nextTrappedErrors.length) { const trappedError = nextTrappedErrors.shift(); const boundary = trappedError.boundary; @@ -570,35 +539,32 @@ module.exports = function(config : HostConfig) { continue; } // Don't visit boundaries twice. - if (knownBoundaries.has(boundary)) { + if (activeErrorBoundaries.has(boundary)) { continue; } try { - knownBoundaries.add(boundary); - + // This error boundary is now active. + // We will let it handle the error and retry rendering. + // If it fails again, the next error will be propagated to the parent + // boundary or rethrown. + activeErrorBoundaries.add(boundary); // Give error boundary a chance to update its state. // Updates will be scheduled with Task priority. - acknowledgeErrorInBoundary(boundary, error); + const instance = boundary.stateNode; + instance.unstable_handleError(error); // Schedule an update, in case the boundary didn't call setState - // on itself + // on itself. scheduleUpdate(boundary); } catch (nextError) { - // If an error is thrown, propagate the error to the next boundary - const te = trapError(boundary, nextError); - if (te.boundary) { - // If a boundary is found, push trapped error onto array - nextTrappedErrors.push(te); - } else { - // Otherwise, we'll need to throw - firstUncaughtError = firstUncaughtError || te.error; - } + // If an error is thrown, propagate the error to the parent boundary. + trapError(boundary, nextError, false); } } - - // Now that we attempt to flush any work that was scheduled by the boundaries - // If this creates errors, they will be pushed to nextTrappedErrors and the loop will continue + // Now that we attempt to flush any work that was scheduled by the boundaries. + // If this creates errors, they will be pushed to nextTrappedErrors and + // the outer loop will continue. try { performTaskWorkUnsafe(true); } catch (error) { @@ -610,26 +576,13 @@ module.exports = function(config : HostConfig) { // should always be set while work is being performed. throw error; } - const trappedError = trapError(failedUnitOfWork, error); - if (nextTrappedErrors) { - nextTrappedErrors.push(trappedError); - } else { - nextTrappedErrors = [trappedError]; - } - } - - - if (!nextTrappedErrors || - !nextTrappedErrors.length || - firstUncaughtError) { - break; + trapError(failedUnitOfWork, error, false); } } nextTrappedErrors = null; - knownBoundaries.clear(); + activeErrorBoundaries = null; priorityContext = previousPriorityContext; - isHandlingErrors = false; if (firstUncaughtError) { // We need to make sure any future root can get scheduled despite these errors. @@ -646,9 +599,15 @@ module.exports = function(config : HostConfig) { while (maybeErrorBoundary) { if (maybeErrorBoundary.tag === ClassComponent) { const instance = maybeErrorBoundary.stateNode; - if (typeof instance.unstable_handleError === 'function' && - !knownBoundaries.has(maybeErrorBoundary)) { - return maybeErrorBoundary; + const isErrorBoundary = typeof instance.unstable_handleError === 'function'; + if (isErrorBoundary) { + const isHandlingAnotherError = ( + activeErrorBoundaries !== null && + activeErrorBoundaries.has(maybeErrorBoundary) + ); + if (!isHandlingAnotherError) { + return maybeErrorBoundary; + } } } maybeErrorBoundary = maybeErrorBoundary.return; @@ -656,16 +615,20 @@ module.exports = function(config : HostConfig) { return null; } - function trapError(fiber : Fiber, error : any) : TrappedError { - return { + function trapError(fiber : Fiber, error : any, isUnmounting : boolean) : void { + if (isUnmounting && activeErrorBoundaries) { + // Ignore errors caused by unmounting during error handling. + // This lets error boundaries safely tear down already failed trees. + return; + } + + if (!nextTrappedErrors) { + nextTrappedErrors = []; + } + nextTrappedErrors.push({ boundary: findClosestErrorBoundary(fiber), error, - }; - } - - function acknowledgeErrorInBoundary(boundary : Fiber, error : any) { - const instance = boundary.stateNode; - instance.unstable_handleError(error); + }); } function scheduleWork(root : FiberRoot) { From e5b76cd511623ab1919f15d0a6a1a4f82dbb231e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Nov 2016 18:32:12 +0000 Subject: [PATCH 07/10] Simplify trapError() calls The existing logic was written before we had a proper error handling system. --- .../shared/fiber/ReactFiberScheduler.js | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index f8f97848892a..6e032e23ff33 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -488,18 +488,7 @@ module.exports = function(config : HostConfig) { break; } } catch (error) { - // Clean-up - ReactCurrentOwner.current = null; - - const failedUnitOfWork = nextUnitOfWork; - // Reset because it points to the error boundary: - nextUnitOfWork = null; - if (!failedUnitOfWork) { - // We shouldn't end up here because nextUnitOfWork - // should always be set while work is being performed. - throw error; - } - trapError(failedUnitOfWork, error, false); + trapError(nextUnitOfWork, error, false); } // If there were any errors, handle them now @@ -568,15 +557,7 @@ module.exports = function(config : HostConfig) { try { performTaskWorkUnsafe(true); } catch (error) { - const failedUnitOfWork = nextUnitOfWork; - // Reset because it points to the error boundary: - nextUnitOfWork = null; - if (!failedUnitOfWork) { - // We shouldn't end up here because nextUnitOfWork - // should always be set while work is being performed. - throw error; - } - trapError(failedUnitOfWork, error, false); + trapError(nextUnitOfWork, error, false); } } @@ -615,7 +596,10 @@ module.exports = function(config : HostConfig) { return null; } - function trapError(fiber : Fiber, error : any, isUnmounting : boolean) : void { + function trapError(fiber : Fiber | null, error : any, isUnmounting : boolean) : void { + // It is no longer valid because we exited the user code. + ReactCurrentOwner.current = null; + if (isUnmounting && activeErrorBoundaries) { // Ignore errors caused by unmounting during error handling. // This lets error boundaries safely tear down already failed trees. @@ -626,7 +610,7 @@ module.exports = function(config : HostConfig) { nextTrappedErrors = []; } nextTrappedErrors.push({ - boundary: findClosestErrorBoundary(fiber), + boundary: fiber ? findClosestErrorBoundary(fiber) : null, error, }); } From f833732e325104c8a0d75152574742e1cdce8453 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Nov 2016 18:37:26 +0000 Subject: [PATCH 08/10] Remove unnecessary flags --- .../shared/fiber/ReactFiberScheduler.js | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 6e032e23ff33..c39faa1f9a96 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -156,7 +156,7 @@ module.exports = function(config : HostConfig) { return null; } - function commitAllWork(finishedWork : Fiber, ignoreUnmountingErrors : boolean) { + function commitAllWork(finishedWork : Fiber) { // Commit all the side-effects within a tree. // First, we'll perform all the host insertions, updates, deletions and // ref unmounts. @@ -191,13 +191,7 @@ module.exports = function(config : HostConfig) { break; case Deletion: case DeletionAndCallback: - // Deletion might cause an error in componentWillUnmount(). - // We will continue nevertheless and handle those later on. - // Note: There is a special case where we completely ignore errors. - // It happens when we already caught an error earlier, and the update - // is caused by an error boundary trying to render an error message. - // In this case, we want to blow away the tree without catching errors. - commitDeletion(effectfulFiber, ignoreUnmountingErrors); + commitDeletion(effectfulFiber); break; } @@ -232,6 +226,7 @@ module.exports = function(config : HostConfig) { commitLifeCycles(current, finishedWork); } + // The task work includes batched updates and error handling. performTaskWork(); } @@ -253,7 +248,7 @@ module.exports = function(config : HostConfig) { workInProgress.pendingWorkPriority = newPriority; } - function completeUnitOfWork(workInProgress : Fiber, ignoreUnmountingErrors : boolean) : ?Fiber { + function completeUnitOfWork(workInProgress : Fiber) : ?Fiber { while (true) { // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here @@ -325,7 +320,7 @@ module.exports = function(config : HostConfig) { // "next" scheduled work since we've already scanned passed. That // also ensures that work scheduled during reconciliation gets deferred. // const hasMoreWork = workInProgress.pendingWorkPriority !== NoWork; - commitAllWork(workInProgress, ignoreUnmountingErrors); + commitAllWork(workInProgress); const nextWork = findNextUnitOfWork(); // if (!nextWork && hasMoreWork) { // TODO: This can happen when some deep work completes and we don't @@ -339,7 +334,7 @@ module.exports = function(config : HostConfig) { } } - function performUnitOfWork(workInProgress : Fiber, ignoreUnmountingErrors : boolean) : ?Fiber { + function performUnitOfWork(workInProgress : Fiber) : ?Fiber { // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in @@ -360,7 +355,7 @@ module.exports = function(config : HostConfig) { ReactFiberInstrumentation.debugTool.onWillCompleteWork(workInProgress); } // If this doesn't spawn new work, complete the current work. - next = completeUnitOfWork(workInProgress, ignoreUnmountingErrors); + next = completeUnitOfWork(workInProgress); if (__DEV__ && ReactFiberInstrumentation.debugTool) { ReactFiberInstrumentation.debugTool.onDidCompleteWork(workInProgress); } @@ -377,7 +372,7 @@ module.exports = function(config : HostConfig) { } while (nextUnitOfWork) { if (deadline.timeRemaining() > timeHeuristicForUnitOfWork) { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); + nextUnitOfWork = performUnitOfWork(nextUnitOfWork); if (!nextUnitOfWork) { // Find more work. We might have time to complete some more. nextUnitOfWork = findNextUnitOfWork(); @@ -399,7 +394,7 @@ module.exports = function(config : HostConfig) { nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === AnimationPriority) { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); + nextUnitOfWork = performUnitOfWork(nextUnitOfWork); if (!nextUnitOfWork) { // Keep searching for animation work until there's no more left nextUnitOfWork = findNextUnitOfWork(); @@ -419,7 +414,7 @@ module.exports = function(config : HostConfig) { nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === SynchronousPriority) { - nextUnitOfWork = performUnitOfWork(nextUnitOfWork, false); + nextUnitOfWork = performUnitOfWork(nextUnitOfWork); if (!nextUnitOfWork) { nextUnitOfWork = findNextUnitOfWork(); @@ -441,12 +436,12 @@ module.exports = function(config : HostConfig) { } } - function performTaskWorkUnsafe(ignoreUnmountingErrors : boolean) { + function performTaskWorkUnsafe() { nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === TaskPriority) { nextUnitOfWork = - performUnitOfWork(nextUnitOfWork, ignoreUnmountingErrors); + performUnitOfWork(nextUnitOfWork); if (!nextUnitOfWork) { nextUnitOfWork = findNextUnitOfWork(); @@ -470,7 +465,7 @@ module.exports = function(config : HostConfig) { performSynchronousWorkUnsafe(); break; case TaskPriority: - performTaskWorkUnsafe(false); + performTaskWorkUnsafe(); break; case AnimationPriority: performAnimationWorkUnsafe(); @@ -555,7 +550,7 @@ module.exports = function(config : HostConfig) { // If this creates errors, they will be pushed to nextTrappedErrors and // the outer loop will continue. try { - performTaskWorkUnsafe(true); + performTaskWorkUnsafe(); } catch (error) { trapError(nextUnitOfWork, error, false); } From 801e347b4f201fd4976f1448d2b36262f35ea7af Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 4 Nov 2016 12:13:49 -0700 Subject: [PATCH 09/10] Prevent performTaskWork recursion --- scripts/fiber/tests-failing.txt | 3 +++ scripts/fiber/tests-passing.txt | 1 - .../dom/shared/ReactDOMFeatureFlags.js | 2 +- .../shared/fiber/ReactFiberScheduler.js | 18 +++++++++++++----- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index c78722ebb6f1..67bbbea71ec0 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate +src/renderers/dom/__tests__/ReactDOMProduction-test.js +* should throw with an error code in production + src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 71f078f40969..b42a892af2cc 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods -* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children diff --git a/src/renderers/dom/shared/ReactDOMFeatureFlags.js b/src/renderers/dom/shared/ReactDOMFeatureFlags.js index 5e9d93267cf9..17bae0a58afb 100644 --- a/src/renderers/dom/shared/ReactDOMFeatureFlags.js +++ b/src/renderers/dom/shared/ReactDOMFeatureFlags.js @@ -13,7 +13,7 @@ var ReactDOMFeatureFlags = { useCreateElement: true, - useFiber: false, + useFiber: true, }; module.exports = ReactDOMFeatureFlags; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index c39faa1f9a96..5a31c8f3f710 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -81,6 +81,9 @@ module.exports = function(config : HostConfig) { // Whether updates should be batched. Only applies when using sync scheduling. let shouldBatchUpdates : boolean = false; + // Need this to prevent recursion while in a Task loop. + let isPerformingTaskWork : boolean = false; + // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; let nextPriorityLevel : PriorityLevel = NoWork; @@ -437,6 +440,7 @@ module.exports = function(config : HostConfig) { } function performTaskWorkUnsafe() { + isPerformingTaskWork = true; nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && nextPriorityLevel === TaskPriority) { @@ -450,6 +454,7 @@ module.exports = function(config : HostConfig) { if (nextUnitOfWork) { scheduleCallbackAtPriority(nextPriorityLevel); } + isPerformingTaskWork = false; } function performTaskWork() { @@ -465,7 +470,11 @@ module.exports = function(config : HostConfig) { performSynchronousWorkUnsafe(); break; case TaskPriority: - performTaskWorkUnsafe(); + if (!isPerformingTaskWork) { + performTaskWorkUnsafe(); + } else { + console.log('prevented recursion!'); + } break; case AnimationPriority: performAnimationWorkUnsafe(); @@ -487,16 +496,14 @@ module.exports = function(config : HostConfig) { } // If there were any errors, handle them now - if (nextTrappedErrors) { + if (nextTrappedErrors && !activeErrorBoundaries) { handleErrors(); } } function handleErrors() : void { - // Prevent recursion. - // TODO: remove the codepath that causes this recursion in the first place. if (activeErrorBoundaries) { - return; + throw new Error('Already handling errors'); } // Start tracking active boundaries. @@ -552,6 +559,7 @@ module.exports = function(config : HostConfig) { try { performTaskWorkUnsafe(); } catch (error) { + isPerformingTaskWork = false; trapError(nextUnitOfWork, error, false); } } From a20eee9912208e6c011d0fcb2b3fc46cbf7d8a54 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 4 Nov 2016 19:35:54 +0000 Subject: [PATCH 10/10] Be stricter about preventing recursion --- scripts/fiber/tests-failing.txt | 3 --- scripts/fiber/tests-passing.txt | 1 + src/renderers/dom/shared/ReactDOMFeatureFlags.js | 2 +- src/renderers/shared/fiber/ReactFiberScheduler.js | 8 +++++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 67bbbea71ec0..c78722ebb6f1 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -87,9 +87,6 @@ src/renderers/art/__tests__/ReactART-test.js * resolves refs before componentDidMount * resolves refs before componentDidUpdate -src/renderers/dom/__tests__/ReactDOMProduction-test.js -* should throw with an error code in production - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should set style attribute when styles exist * should warn when using hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index b42a892af2cc..71f078f40969 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -438,6 +438,7 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js * should use prod React * should handle a simple flow * should call lifecycle methods +* should throw with an error code in production src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js * should render strings as children diff --git a/src/renderers/dom/shared/ReactDOMFeatureFlags.js b/src/renderers/dom/shared/ReactDOMFeatureFlags.js index 17bae0a58afb..5e9d93267cf9 100644 --- a/src/renderers/dom/shared/ReactDOMFeatureFlags.js +++ b/src/renderers/dom/shared/ReactDOMFeatureFlags.js @@ -13,7 +13,7 @@ var ReactDOMFeatureFlags = { useCreateElement: true, - useFiber: true, + useFiber: false, }; module.exports = ReactDOMFeatureFlags; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 5a31c8f3f710..dfd1fe4cdcb8 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -440,6 +440,10 @@ module.exports = function(config : HostConfig) { } function performTaskWorkUnsafe() { + if (isPerformingTaskWork) { + throw new Error('Already performing task work'); + } + isPerformingTaskWork = true; nextUnitOfWork = findNextUnitOfWork(); while (nextUnitOfWork && @@ -472,8 +476,6 @@ module.exports = function(config : HostConfig) { case TaskPriority: if (!isPerformingTaskWork) { performTaskWorkUnsafe(); - } else { - console.log('prevented recursion!'); } break; case AnimationPriority: @@ -495,7 +497,7 @@ module.exports = function(config : HostConfig) { trapError(nextUnitOfWork, error, false); } - // If there were any errors, handle them now + // If there were errors and we aren't already handling them, handle them now if (nextTrappedErrors && !activeErrorBoundaries) { handleErrors(); }