Skip to content

Commit

Permalink
Fix the bug when switching to a null portal child
Browse files Browse the repository at this point in the history
If pendingProps is null, we do a bailout in beginWork.
This prevents unmounting of the existing child when the new child is null.

We fix this by changing portal fiber's pendingProps to be the portal object itself instead of its children.
This way, it is never null, and thus doesn't cause a false positive in the bailout condition.
  • Loading branch information
gaearon committed Dec 9, 2016
1 parent 3f2129a commit ed9747d
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 7 deletions.
3 changes: 0 additions & 3 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js
src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should throw with an error code in production

src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should reconcile portal children

src/renderers/dom/shared/__tests__/ReactDOM-test.js
* throws in render() if the mount callback is not a function
* throws in render() if the update callback is not a function
Expand Down
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render one portal
* should render many portals
* should render nested portals
* should reconcile portal children
* should keep track of namespace across portals (simple)
* should keep track of namespace across portals (medium)
* should keep track of namespace across portals (complex)
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/shared/fiber/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
} else {
// Update
const existing = useFiber(current, priority);
existing.pendingProps = portal.children;
existing.pendingProps = portal;
existing.return = returnFiber;
return existing;
}
Expand Down Expand Up @@ -976,7 +976,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, priority);
existing.pendingProps = portal.children;
existing.pendingProps = portal;
existing.return = returnFiber;
return existing;
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ exports.createFiberFromYield = function(yieldNode : ReactYield, priorityLevel :

exports.createFiberFromPortal = function(portal : ReactPortal, priorityLevel : PriorityLevel) : Fiber {
const fiber = createFiber(HostPortal, portal.key);
fiber.pendingProps = portal.children;
fiber.pendingProps = portal;
fiber.pendingWorkPriority = priorityLevel;
fiber.stateNode = {
containerInfo: portal.containerInfo,
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ module.exports = function<T, P, I, TI, C, CX>(

function updatePortalComponent(current, workInProgress) {
const priorityLevel = workInProgress.pendingWorkPriority;
const nextChildren = workInProgress.pendingProps;
const nextChildren = workInProgress.pendingProps.children;
if (!current) {
// Portals are special because we don't append the children during mount
// but at commit. Therefore we need to track insertions which the normal
Expand Down

0 comments on commit ed9747d

Please sign in to comment.