Delete child when the key lines up but the type doesn't (#8085)
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.
Sebastian Markbåge committed
Oct 25, 2016 at 07:51 UTC
ea8cf7fbff210711f5121fdb0f0ee008298bef1a
2 files changed
+126
-1
src/renderers/shared/fiber/ReactChildFiber.js
+8
-1
@@ -508,6 +508,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
508
// boolean, undefined, etc.
509
break;
510
}
511
+ if (shouldTrackSideEffects) {
512
+ if (oldFiber && !newFiber.alternate) {
513
+ // We matched the slot, but we didn't reuse the existing fiber, so we
514
+ // need to delete the existing child.
515
+ deleteChild(returnFiber, oldFiber);
516
+ }
517
+ }
518
lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
519
if (!previousNewFiber) {
520
// TODO: Move out of the loop. This only happens for the first run.
@@ -573,7 +580,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
580
// it from the child list so that we don't add it to the deletion
581
// list.
582
existingChildren.delete(
576
- newFiber.key === null ? newFiber.index : newFiber.key
583
+ newFiber.key === null ? newIdx : newFiber.key
584
);
585
}
586
}
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+118
@@ -164,6 +164,124 @@ describe('ReactIncrementalSideEffects', () => {
164
165
});
166
167
+ it('can delete a child that changes type - implicit keys', function() {
168
+
169
+ let unmounted = false;
170
+
171
+ class ClassComponent extends React.Component {
172
+ componentWillUnmount() {
173
+ unmounted = true;
174
+ }
175
+ render() {
176
+ return <span prop="Class" />;
177
+ }
178
+ }
179
+
180
+ function FunctionalComponent(props) {
181
+ return <span prop="Function" />;
182
+ }
183
+
184
+ function Foo(props) {
185
+ return (
186
+ <div>
187
+ {props.useClass ?
188
+ <ClassComponent /> :
189
+ props.useFunction ?
190
+ <FunctionalComponent /> :
191
+ props.useText ?
192
+ 'Text' :
193
+ null
194
+ }
195
+ Trail
196
+ </div>
197
+ );
198
+ }
199
+
200
+ ReactNoop.render(<Foo useClass={true} />);
201
+ ReactNoop.flush();
202
+ expect(ReactNoop.root.children).toEqual([
203
+ div(span('Class'), 'Trail'),
204
+ ]);
205
+
206
+ expect(unmounted).toBe(false);
207
+
208
+ ReactNoop.render(<Foo useFunction={true} />);
209
+ ReactNoop.flush();
210
+ expect(ReactNoop.root.children).toEqual([
211
+ div(span('Function'), 'Trail'),
212
+ ]);
213
+
214
+ expect(unmounted).toBe(true);
215
+
216
+ ReactNoop.render(<Foo useText={true} />);
217
+ ReactNoop.flush();
218
+ expect(ReactNoop.root.children).toEqual([
219
+ div('Text', 'Trail'),
220
+ ]);
221
+
222
+ ReactNoop.render(<Foo />);
223
+ ReactNoop.flush();
224
+ expect(ReactNoop.root.children).toEqual([
225
+ div('Trail'),
226
+ ]);
227
+
228
+ });
229
+
230
+ it('can delete a child that changes type - explicit keys', function() {
231
+
232
+ let unmounted = false;
233
+
234
+ class ClassComponent extends React.Component {
235
+ componentWillUnmount() {
236
+ unmounted = true;
237
+ }
238
+ render() {
239
+ return <span prop="Class" />;
240
+ }
241
+ }
242
+
243
+ function FunctionalComponent(props) {
244
+ return <span prop="Function" />;
245
+ }
246
+
247
+ function Foo(props) {
248
+ return (
249
+ <div>
250
+ {props.useClass ?
251
+ <ClassComponent key="a" /> :
252
+ props.useFunction ?
253
+ <FunctionalComponent key="a" /> :
254
+ null
255
+ }
256
+ Trail
257
+ </div>
258
+ );
259
+ }
260
+
261
+ ReactNoop.render(<Foo useClass={true} />);
262
+ ReactNoop.flush();
263
+ expect(ReactNoop.root.children).toEqual([
264
+ div(span('Class'), 'Trail'),
265
+ ]);
266
+
267
+ expect(unmounted).toBe(false);
268
+
269
+ ReactNoop.render(<Foo useFunction={true} />);
270
+ ReactNoop.flush();
271
+ expect(ReactNoop.root.children).toEqual([
272
+ div(span('Function'), 'Trail'),
273
+ ]);
274
+
275
+ expect(unmounted).toBe(true);
276
+
277
+ ReactNoop.render(<Foo />);
278
+ ReactNoop.flush();
279
+ expect(ReactNoop.root.children).toEqual([
280
+ div('Trail'),
281
+ ]);
282
+
283
+ });
284
+
285
it('does not update child nodes if a flush is aborted', () => {
286
287
function Bar(props) {