[Fiber] Remove output field

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/ReactChildFiber.js

@@ -294,14 +294,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
       // Insert
       const reifiedYield = createReifiedYield(yieldNode);
       const created = createFiberFromYield(yieldNode, priority);
-      created.output = reifiedYield;
+      created.type = reifiedYield;
       created.return = returnFiber;
       return created;
     } else {
       // Move based on index
       const existing = useFiber(current, priority);
-      existing.output = createUpdatedReifiedYield(
-        current.output,
+      existing.type = createUpdatedReifiedYield(
+        current.type,
         yieldNode
       );
       existing.return = returnFiber;
@@ -386,7 +386,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         case REACT_YIELD_TYPE: {
           const reifiedYield = createReifiedYield(newChild);
           const created = createFiberFromYield(newChild, priority);
-          created.output = reifiedYield;
+          created.type = reifiedYield;
           created.return = returnFiber;
           return created;
         }
@@ -790,8 +790,8 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         if (child.tag === YieldComponent) {
           deleteRemainingChildren(returnFiber, child.sibling);
           const existing = useFiber(child, priority);
-          existing.output = createUpdatedReifiedYield(
-            child.output,
+          existing.type = createUpdatedReifiedYield(
+            child.type,
             yieldNode
           );
           existing.return = returnFiber;
@@ -808,7 +808,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
 
     const reifiedYield = createReifiedYield(yieldNode);
     const created = createFiberFromYield(yieldNode, priority);
-    created.output = reifiedYield;
+    created.type = reifiedYield;
     created.return = returnFiber;
     return created;
   }

src/renderers/shared/fiber/ReactFiber.js

@@ -97,9 +97,6 @@ export type Fiber = {
   memoizedState: any,
   // Linked list of callbacks to call after updates are committed.
   callbackList: ?UpdateQueue,
-  // Output is the return value of this fiber, or a linked list of return values
-  // if this returns multiple values. Such as a fragment.
-  output: any, // This type will be more specific once we overload the tag.
 
   // Effect
   effectTag: TypeOfSideEffect,
@@ -190,7 +187,6 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
     updateQueue: null,
     memoizedState: null,
     callbackList: null,
-    output: null,
 
     effectTag: NoEffect,
     nextEffect: null,
@@ -267,7 +263,6 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
 
   alt.memoizedProps = fiber.memoizedProps;
   alt.memoizedState = fiber.memoizedState;
-  alt.output = fiber.output;
 
   return alt;
 };

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/ReactFiberCompleteWork.js

@@ -62,28 +62,31 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     workInProgress.effectTag |= Callback;
   }
 
-  function transferOutput(child : ?Fiber, returnFiber : Fiber) {
-    // If we have a single result, we just pass that through as the output to
-    // avoid unnecessary traversal. When we have multiple output, we just pass
-    // the linked list of fibers that has the individual output values.
-    returnFiber.output = (child && !child.sibling) ? child.output : child;
-    returnFiber.memoizedProps = returnFiber.pendingProps;
-  }
-
-  function recursivelyFillYields(yields, output : ?Fiber | ?ReifiedYield) {
-    if (!output) {
-      // Ignore nulls etc.
-    } else if (output.tag !== undefined) { // TODO: Fix this fragile duck test.
-      // Detect if this is a fiber, if so it is a fragment result.
-      // $FlowFixMe: Refinement issue.
-      var item = (output : Fiber);
-      do {
-        recursivelyFillYields(yields, item.output);
-        item = item.sibling;
-      } while (item);
-    } else {
-      // $FlowFixMe: Refinement issue. If it is not a Fiber or null, it is a yield
-      yields.push(output);
+  function appendAllYields(yields : Array<ReifiedYield>, workInProgress : Fiber) {
+    let node = workInProgress.child;
+    while (node) {
+      if (node.tag === HostComponent || node.tag === HostText ||
+          node.tag === Portal) {
+        throw new Error('A coroutine cannot have host component children.');
+      } else if (node.tag === YieldComponent) {
+        yields.push(node.type);
+      } else if (node.child) {
+        // TODO: Coroutines need to visit the stateNode.
+        node.child.return = node;
+        node = node.child;
+        continue;
+      }
+      if (node === workInProgress) {
+        return;
+      }
+      while (!node.sibling) {
+        if (!node.return || node.return === workInProgress) {
+          return;
+        }
+        node = node.return;
+      }
+      node.sibling.return = node.return;
+      node = node.sibling;
     }
   }
 
@@ -105,11 +108,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
     // Build up the yields.
     // TODO: Compare this to a generator or opaque helpers like Children.
     var yields : Array<ReifiedYield> = [];
-    var child = workInProgress.child;
-    while (child) {
-      recursivelyFillYields(yields, child.output);
-      child = child.sibling;
-    }
+    appendAllYields(yields, workInProgress);
     var fn = coroutine.handler;
     var props = coroutine.props;
     var nextChildren = fn(props, yields);
@@ -158,10 +157,9 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
   function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
     switch (workInProgress.tag) {
       case FunctionalComponent:
-        transferOutput(workInProgress.child, workInProgress);
+        workInProgress.memoizedProps = workInProgress.pendingProps;
         return null;
       case ClassComponent:
-        transferOutput(workInProgress.child, workInProgress);
         // We are leaving this subtree, so pop context if any.
         if (isContextProvider(workInProgress)) {
           popContextProvider();
@@ -191,7 +189,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         }
         return null;
       case HostContainer: {
-        transferOutput(workInProgress.child, workInProgress);
+        workInProgress.memoizedProps = workInProgress.pendingProps;
         popContextProvider();
         const fiberRoot = (workInProgress.stateNode : FiberRoot);
         if (fiberRoot.pendingContext) {
@@ -221,8 +219,6 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
             // This returns true if there was something to update.
             markUpdate(workInProgress);
           }
-          // TODO: Is this actually ever going to change? Why set it every time?
-          workInProgress.output = instance;
         } else {
           if (!newProps) {
             if (workInProgress.stateNode === null) {
@@ -242,9 +238,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
           appendAllChildren(instance, workInProgress);
           finalizeInitialChildren(instance, workInProgress.type, newProps);
 
-          // TODO: This seems like unnecessary duplication.
           workInProgress.stateNode = instance;
-          workInProgress.output = instance;
           if (workInProgress.ref) {
             // If there is a ref on a host node we need to schedule a callback
             markUpdate(workInProgress);
@@ -268,29 +262,26 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
             }
           }
           const textInstance = createTextInstance(newText, workInProgress);
-          // TODO: This seems like unnecessary duplication.
           workInProgress.stateNode = textInstance;
-          workInProgress.output = textInstance;
         }
         workInProgress.memoizedProps = newText;
         return null;
       case CoroutineComponent:
         return moveCoroutineToHandlerPhase(current, workInProgress);
       case CoroutineHandlerPhase:
-        transferOutput(workInProgress.stateNode, workInProgress);
+        workInProgress.memoizedProps = workInProgress.pendingProps;
         // Reset the tag to now be a first phase coroutine.
         workInProgress.tag = CoroutineComponent;
         return null;
       case YieldComponent:
         // Does nothing.
         return null;
       case Fragment:
-        transferOutput(workInProgress.child, workInProgress);
+        workInProgress.memoizedProps = workInProgress.pendingProps;
         return null;
       case Portal:
         // TODO: Only mark this as an update if we have any pending callbacks.
         markUpdate(workInProgress);
-        workInProgress.output = null;
         workInProgress.memoizedProps = workInProgress.pendingProps;
         return null;
 

src/renderers/shared/fiber/ReactFiberReconciler.js

@@ -14,7 +14,6 @@
 
 import type { Fiber } from 'ReactFiber';
 import type { FiberRoot } from 'ReactFiberRoot';
-import type { TypeOfWork } from 'ReactTypeOfWork';
 import type { PriorityLevel } from 'ReactPriorityLevel';
 
 var {
@@ -39,10 +38,6 @@ export type Deadline = {
   timeRemaining : () => number
 };
 
-type HostChildNode<I> = { tag: TypeOfWork, output: HostChildren<I>, sibling: any };
-
-export type HostChildren<I> = null | void | I | HostChildNode<I>;
-
 type OpaqueNode = Fiber;
 
 export type HostConfig<T, P, I, TI, C> = {

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',
+    ]);
+
+  });
 });