Unmount child from the first phase of a coroutine

Author: sebmarkbage

scripts/fiber/tests-passing.txt

@@ -1031,6 +1031,7 @@ src/renderers/shared/__tests__/ReactPerf-test.js
 
 src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js
 * should render a coroutine
+* should unmount a composite in a coroutine
 
 src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
 * should render a simple component

src/renderers/shared/fiber/ReactFiberCommitWork.js

@@ -21,6 +21,7 @@ var {
   HostContainer,
   HostComponent,
   HostText,
+  CoroutineComponent,
   Portal,
 } = ReactTypeOfWork;
 var { callCallbacks } = require('ReactFiberUpdateQueue');
@@ -278,6 +279,10 @@ module.exports = function<T, P, I, TI, C>(
         detachRef(current);
         return;
       }
+      case CoroutineComponent: {
+        commitNestedUnmounts(current.stateNode);
+        return;
+      }
     }
   }
 

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -286,6 +286,12 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
       workInProgress.pendingProps = null;
       workInProgress.updateQueue = null;
 
+      if (next) {
+        // If completing this work spawned new work, do that next. We'll come
+        // back here again.
+        return next;
+      }
+
       const returnFiber = workInProgress.return;
 
       if (returnFiber) {
@@ -318,10 +324,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         }
       }
 
-      if (next) {
-        // If completing this work spawned new work, do that next.
-        return next;
-      } else if (workInProgress.sibling) {
+      if (workInProgress.sibling) {
         // If there is more work to do in this returnFiber, do that next.
         return workInProgress.sibling;
       } else if (returnFiber) {

src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js

@@ -24,10 +24,8 @@ describe('ReactCoroutine', () => {
   });
 
   it('should render a coroutine', () => {
-
     var ops = [];
 
-
     function Continuation({ isSame }) {
       ops.push(['Continuation', isSame]);
       return <span>{isSame ? 'foo==bar' : 'foo!=bar'}</span>;
@@ -84,7 +82,72 @@ describe('ReactCoroutine', () => {
       ['Continuation', true],
       ['Continuation', false],
     ]);
-
   });
 
+  it('should unmount a composite in a coroutine', () => {
+    var ops = [];
+
+    class Continuation extends React.Component {
+      render() {
+        ops.push('Continuation');
+        return <div />;
+      }
+      componentWillUnmount() {
+        ops.push('Unmount Continuation');
+      }
+    }
+
+    class Child extends React.Component {
+      render() {
+        ops.push('Child');
+        return ReactCoroutine.createYield({}, Continuation, null);
+      }
+      componentWillUnmount() {
+        ops.push('Unmount Child');
+      }
+    }
+
+    function HandleYields(props, yields) {
+      ops.push('HandleYields');
+      return yields.map(y => <y.continuation />);
+    }
+
+    class Parent extends React.Component {
+      render() {
+        ops.push('Parent');
+        return ReactCoroutine.createCoroutine(
+          this.props.children,
+          HandleYields,
+          this.props
+        );
+      }
+      componentWillUnmount() {
+        ops.push('Unmount Parent');
+      }
+    }
+
+    ReactNoop.render(<Parent><Child /></Parent>);
+    ReactNoop.flush();
+
+    expect(ops).toEqual([
+      'Parent',
+      'Child',
+      'HandleYields',
+      'Continuation',
+    ]);
+
+    ops = [];
+
+    ReactNoop.render(<div />);
+    ReactNoop.flush();
+
+    expect(ops).toEqual([
+      'Unmount Parent',
+      // TODO: This should happen in the order Child, Continuation which it
+      // will once we swap stateNode and child positions of these.
+      'Unmount Continuation',
+      'Unmount Child',
+    ]);
+
+  });
 });