Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d9393ee
Add Fragment fiber type
sebmarkbage Sep 13, 2016
e8c1cb4
Add Text node types
sebmarkbage Sep 14, 2016
d7322d8
Silence Fiber warning when the feature flag is on
sebmarkbage Sep 13, 2016
e05ab67
Fix MultiChild tests so they work with Fiber
sebmarkbage Sep 13, 2016
8c7409b
Add comment about bug in yields
sebmarkbage Sep 15, 2016
4ed67f1
Enable text updates in ReactNoop
sebmarkbage Sep 19, 2016
dff0faf
Fiber child reconciliation
sebmarkbage Sep 20, 2016
51f2bf9
Add index field to each fiber
sebmarkbage Sep 21, 2016
bcbceae
Don't track side-effects unless needed
sebmarkbage Sep 21, 2016
c49a91c
Fast path for create child
sebmarkbage Oct 4, 2016
be0acf8
Deletion tracking
sebmarkbage Oct 4, 2016
76725fe
Tag the fiber with the kind of side-effect that was applied to it
sebmarkbage Oct 4, 2016
86e854e
Append deletions to the effect list
sebmarkbage Oct 4, 2016
eabed69
Move child updates to use the reconciled effects
sebmarkbage Oct 5, 2016
31ca1e7
Remove beginWork shortcut
sebmarkbage Oct 6, 2016
0262e70
Reset effect list when we recompute children
sebmarkbage Oct 7, 2016
f3d7116
Always override priority level when visiting children
sebmarkbage Oct 7, 2016
40989f8
Call componentWillUnmount during deletion phase
sebmarkbage Oct 7, 2016
8360088
Fire componentDidMount/componentDidUpdate life-cycles
sebmarkbage Oct 7, 2016
48cf81c
Resolve ref callbacks
sebmarkbage Oct 7, 2016
8721ec1
Invoke all null ref calls before any new ref calls
sebmarkbage Oct 7, 2016
d9efde7
Fix resuming bug
sebmarkbage Oct 10, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add index field to each fiber
We use this to track the index slot that this Fiber had at
reconciliation time. We will use this for two purposes:

1) We use it to quickly determine if a move is needed.

2) We also use it to model a sparce linked list, since we can have
null/false/undefined in our child set and these take up a slot for
the implicit key.
  • Loading branch information
sebmarkbage committed Oct 17, 2016
commit 51f2bf998a8e3841a6d709595dd93809df9d2f8f
94 changes: 76 additions & 18 deletions src/renderers/shared/fiber/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,22 @@ function deleteChild(
returnFiber : Fiber,
childToDelete : Fiber
) {
if (!shouldTrackSideEffects) {
// Noop.
return;
}

// TODO: Add this child to the side-effect queue for deletion.
}

function deleteRemainingChildren(
returnFiber : Fiber,
currentFirstChild : ?Fiber
) {
if (!shouldTrackSideEffects) {
// Noop.
return null;
}
// TODO: Add these children to the side-effect queue for deletion.
return null;
}
Expand All @@ -81,10 +90,6 @@ function mapAndDeleteRemainingChildren(
// we will then undo the deletion as we restore children. Implicit (null) keys
// don't get added to this set.
const existingChildren : Map<string, Fiber> = new Map();
// TODO: This also needs to store the "previous index of this node". That lets
// us determine whether something needs to be a placement. It might be best to
// just store this on the fiber itself since that lets us use the "implicit"
// index resolution mechanism without adding null values to the linked list.
let existingChild = currentFirstChild;
while (existingChild) {
if (existingChild.key !== null) {
Expand All @@ -109,22 +114,48 @@ function mapAndDeleteRemainingChildren(
function ChildReconciler(shouldClone, shouldTrackSideEffects) {

function useFiber(fiber : Fiber, priority : PriorityLevel) {
// We currently set sibling to null here because it is easy to forget to do
// before returning it.
// We currently set sibling to null and index to 0 here because it is easy
// to forget to do before returning it. E.g. for the single child case.
if (shouldClone) {
const clone = cloneFiber(fiber, priority);
clone.index = 0;
clone.sibling = null;
return clone;
} else {
if (fiber.pendingWorkPriority === NoWork ||
fiber.pendingWorkPriority > priority) {
fiber.pendingWorkPriority = priority;
}
fiber.index = 0;
fiber.sibling = null;
return fiber;
}
}

function placeChild(newFiber : Fiber, lastPlacedIndex : number, newIndex : number) {
newFiber.index = newIndex;
if (!shouldTrackSideEffects) {
// Noop.
return lastPlacedIndex;
}
const current = newFiber.alternate;
if (current) {
const oldIndex = current.index;
if (oldIndex < lastPlacedIndex) {
// This is a move.
// TODO: Schedule a move side-effect for this child.
return lastPlacedIndex;
} else {
// This item can stay in place.
return oldIndex;
}
} else {
// This is an insertion.
// TODO: Schedule an insertion side-effect for this child.
return lastPlacedIndex;
}
}

function updateTextNode(
returnFiber : Fiber,
current : ?Fiber,
Expand Down Expand Up @@ -234,17 +265,19 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {

function updateSlot(
returnFiber : Fiber,
oldFiber : Fiber,
oldFiber : ?Fiber,
newChild : any,
priority : PriorityLevel
) : ?Fiber {
// Update the fiber if the keys match, otherwise return null.

const key = oldFiber ? oldFiber.key : null;

if (typeof newChild === 'string' || typeof newChild === 'number') {
// Text nodes doesn't have keys. If the previous node is implicitly keyed
// we can continue to replace it without aborting even if it is not a text
// node.
if (oldFiber.key !== null) {
if (key !== null) {
return null;
}
return updateTextNode(returnFiber, oldFiber, '' + newChild, priority);
Expand All @@ -253,7 +286,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
if (typeof newChild === 'object' && newChild !== null) {
switch (newChild.$$typeof) {
case REACT_ELEMENT_TYPE: {
if (newChild.key === oldFiber.key) {
if (newChild.key === key) {
return updateElement(
returnFiber,
oldFiber,
Expand All @@ -266,7 +299,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
}

case REACT_COROUTINE_TYPE: {
if (newChild.key === oldFiber.key) {
if (newChild.key === key) {
return updateCoroutine(
returnFiber,
oldFiber,
Expand All @@ -279,7 +312,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
}

case REACT_YIELD_TYPE: {
if (newChild.key === oldFiber.key) {
if (newChild.key === key) {
return updateYield(
returnFiber,
oldFiber,
Expand All @@ -295,7 +328,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
if (isArray(newChild) || getIteratorFn(newChild)) {
// Fragments doesn't have keys so if the previous key is implicit we can
// update it.
if (oldFiber.key !== null) {
if (key !== null) {
return null;
}
return updateFragment(returnFiber, oldFiber, newChild, priority);
Expand Down Expand Up @@ -433,13 +466,32 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
let previousNewFiber : ?Fiber = null;

let oldFiber = currentFirstChild;
let lastPlacedIndex = 0;
let newIdx = 0;
let nextOldFiber = null;
for (; oldFiber && newIdx < newChildren.length; newIdx++) {
const nextOldFiber = oldFiber.sibling; // In-case we mutate this fiber.
const newFiber = updateSlot(returnFiber, oldFiber, newChildren[newIdx], priority);
if (oldFiber) {
if (oldFiber.index > newIdx) {
nextOldFiber = oldFiber;
oldFiber = null;
} else {
nextOldFiber = oldFiber.sibling;
}
}
const newFiber = updateSlot(
returnFiber,
oldFiber,
newChildren[newIdx],
priority
);
if (!newFiber) {
// TODO: This breaks on empty slots like null children. That's
// unfortunate because it triggers the slow path all the time. We need
// a better way to communicate whether this was a miss or null,
// boolean, undefined, etc.
break;
}
lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
if (!previousNewFiber) {
// TODO: Move out of the loop. This only happens for the first run.
resultingFirstChild = newFiber;
Expand Down Expand Up @@ -467,7 +519,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
for (; newIdx < newChildren.length; newIdx++) {
// TODO: Since the mutation of existing fibers can happen at any order
// we might break the link before we're done with it. :(
const nextOldFiber = oldFiber ? oldFiber.sibling : null;
if (oldFiber) {
if (oldFiber.index > newIdx) {
nextOldFiber = oldFiber;
oldFiber = null;
} else {
nextOldFiber = oldFiber.sibling;
}
}
const newFiber = updateFromMap(
existingChildren,
returnFiber,
Expand All @@ -476,6 +535,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
priority
);
if (newFiber) {
lastPlacedIndex = placeChild(newFiber, lastPlacedIndex, newIdx);
if (!previousNewFiber) {
resultingFirstChild = newFiber;
} else {
Expand All @@ -485,9 +545,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
}
// We will keep traversing the oldFiber in order, in case the new child
// has a null key that we'll need to match in the same slot.
if (oldFiber) {
oldFiber = nextOldFiber;
}
oldFiber = nextOldFiber;
}

// TODO: Add deletion side-effects to the returnFiber's side-effects.
Expand Down
3 changes: 3 additions & 0 deletions src/renderers/shared/fiber/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export type Fiber = Instance & {
// Singly Linked List Tree Structure.
child: ?Fiber,
sibling: ?Fiber,
index: number,

// The ref last used to attach this node.
// I'll avoid adding an owner field for prod and model that as functions.
Expand Down Expand Up @@ -156,6 +157,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {

child: null,
sibling: null,
index: 0,

ref: null,

Expand Down Expand Up @@ -221,6 +223,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
alt.stateNode = fiber.stateNode;
alt.child = fiber.child;
alt.sibling = fiber.sibling; // This should always be overridden. TODO: null
alt.index = fiber.index; // This should always be overridden.
alt.ref = fiber.ref;
// pendingProps is here for symmetry but is unnecessary in practice for now.
// TODO: Pass in the new pendingProps as an argument maybe?
Expand Down
69 changes: 69 additions & 0 deletions src/renderers/shared/fiber/__tests__/ReactTopLevelFragment-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,73 @@ describe('ReactTopLevelFragment', function() {

});

it('preserves state if an implicit key slot switches from/to null', function() {

var instance = null;

class Stateful extends React.Component {
render() {
instance = this;
return <div>World</div>;
}
}

function Fragment({ condition }) {
return condition ? [null, <Stateful />] :
[<div>Hello</div>, <Stateful />];
}
ReactNoop.render(<Fragment />);
ReactNoop.flush();

var instanceA = instance;

expect(instanceA).not.toBe(null);

ReactNoop.render(<Fragment condition={true} />);
ReactNoop.flush();

var instanceB = instance;

expect(instanceB).toBe(instanceA);

ReactNoop.render(<Fragment condition={false} />);
ReactNoop.flush();

var instanceC = instance;

expect(instanceC === instanceA).toBe(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could write this as expect(instanceC).toBe(instanceA)?

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Sep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I can't be cause when it errors that stalls jest trying to print the error presumably. I spent some time trying to figure out why but the debugger doesn't work on it. My best bet is that a message simply gets dropped which stalls the process waiting for a response that never arrives. cc @cpojer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not good. Would you mind opening an issue about this on Jest's bugtracker?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


});

it('should preserve state in a reorder', function() {

var instance = null;

class Stateful extends React.Component {
render() {
instance = this;
return <div>Hello</div>;
}
}

function Fragment({ condition }) {
return condition ? [[<div key="b">World</div>, <Stateful key="a" />]] :
[[<Stateful key="a" />, <div key="b">World</div>], <div />];
}
ReactNoop.render(<Fragment />);
ReactNoop.flush();

var instanceA = instance;

expect(instanceA).not.toBe(null);

ReactNoop.render(<Fragment condition={true} />);
ReactNoop.flush();

var instanceB = instance;

expect(instanceB).toBe(instanceA);

});

});