From a6fdaa41a6a5ae5b2ed316e1a0da407038fd342a Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sema@fb.com>
Date: Mon, 24 Oct 2016 18:27:51 -0700
Subject: [PATCH] Delete child when the key lines up but the type doesn't

When keys line up in the beginning but the type doesn't
we don't currently delete the child. We need to track that this
fiber is a not a reuse and if so mark the old one for deletion.
---
 src/renderers/shared/fiber/ReactChildFiber.js |   9 +-
 .../ReactIncrementalSideEffects-test.js       | 118 ++++++++++++++++++
 2 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js
index af4af1540456..f51a00ba011e 100644
--- a/src/renderers/shared/fiber/ReactChildFiber.js
+++ b/src/renderers/shared/fiber/ReactChildFiber.js
@@ -508,6 +508,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
         // boolean, undefined, etc.
         break;
       }
+      if (shouldTrackSideEffects) {
+        if (oldFiber && !newFiber.alternate) {
+          // We matched the slot, but we didn't reuse the existing fiber, so we
+          // need to delete the existing child.
+          deleteChild(returnFiber, oldFiber);
+        }
+      }
       lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
       if (!previousNewFiber) {
         // TODO: Move out of the loop. This only happens for the first run.
@@ -573,7 +580,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
             // it from the child list so that we don't add it to the deletion
             // list.
             existingChildren.delete(
-              newFiber.key === null ? newFiber.index : newFiber.key
+              newFiber.key === null ? newIdx : newFiber.key
             );
           }
         }
diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
index 897422d2274d..ab8f7a55d7cb 100644
--- a/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
@@ -164,6 +164,124 @@ describe('ReactIncrementalSideEffects', () => {
 
   });
 
+  it('can delete a child that changes type - implicit keys', function() {
+
+    let unmounted = false;
+
+    class ClassComponent extends React.Component {
+      componentWillUnmount() {
+        unmounted = true;
+      }
+      render() {
+        return <span prop="Class" />;
+      }
+    }
+
+    function FunctionalComponent(props) {
+      return <span prop="Function" />;
+    }
+
+    function Foo(props) {
+      return (
+        <div>
+          {props.useClass ?
+            <ClassComponent /> :
+            props.useFunction ?
+            <FunctionalComponent /> :
+            props.useText ?
+            'Text' :
+            null
+          }
+          Trail
+        </div>
+      );
+    }
+
+    ReactNoop.render(<Foo useClass={true} />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div(span('Class'), 'Trail'),
+    ]);
+
+    expect(unmounted).toBe(false);
+
+    ReactNoop.render(<Foo useFunction={true} />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div(span('Function'), 'Trail'),
+    ]);
+
+    expect(unmounted).toBe(true);
+
+    ReactNoop.render(<Foo useText={true} />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div('Text', 'Trail'),
+    ]);
+
+    ReactNoop.render(<Foo />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div('Trail'),
+    ]);
+
+  });
+
+  it('can delete a child that changes type - explicit keys', function() {
+
+    let unmounted = false;
+
+    class ClassComponent extends React.Component {
+      componentWillUnmount() {
+        unmounted = true;
+      }
+      render() {
+        return <span prop="Class" />;
+      }
+    }
+
+    function FunctionalComponent(props) {
+      return <span prop="Function" />;
+    }
+
+    function Foo(props) {
+      return (
+        <div>
+          {props.useClass ?
+            <ClassComponent key="a" /> :
+            props.useFunction ?
+            <FunctionalComponent key="a" /> :
+            null
+          }
+          Trail
+        </div>
+      );
+    }
+
+    ReactNoop.render(<Foo useClass={true} />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div(span('Class'), 'Trail'),
+    ]);
+
+    expect(unmounted).toBe(false);
+
+    ReactNoop.render(<Foo useFunction={true} />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div(span('Function'), 'Trail'),
+    ]);
+
+    expect(unmounted).toBe(true);
+
+    ReactNoop.render(<Foo />);
+    ReactNoop.flush();
+    expect(ReactNoop.root.children).toEqual([
+      div('Trail'),
+    ]);
+
+  });
+
   it('does not update child nodes if a flush is aborted', () => {
 
     function Bar(props) {