Attack of the Clones!
This fixes some bugs with the clones and traversing backwards through them. It is important that we maintain the correct parent at all times and that clones have the correct values. We need to carefully clone everything on the way up to the the fiber with the next work to do. This code is a bit messy and fragile now. I'm sure I didn't get it all right but I want to get the basics in place first. Then we can structure this part better. I think the general algorithm is sound though.
Sebastian Markbage committed
Jun 16, 2016 at 18:21 UTC
97cd8e179aea9a12e343af6864e0e5994e40a8fc
5 files changed
+69
-34
src/renderers/shared/fiber/ReactChildFiber.js
+2
@@ -46,6 +46,7 @@ function createSubsequentChild(parent : Fiber, existingChild : ?Fiber, previousS
46
clone.pendingProps = element.props;
47
clone.child = existingChild.child;
48
clone.sibling = null;
49
+ clone.parent = parent;
50
previousSibling.sibling = clone;
51
return clone;
52
}
@@ -108,6 +109,7 @@ function createFirstChild(parent, existingChild, newChildren, priority) {
109
clone.pendingProps = element.props;
110
clone.child = existingChild.child;
111
clone.sibling = null;
112
+ clone.parent = parent;
113
return clone;
114
}
115
const child = ReactFiber.createFiberFromElement(element, priority);
src/renderers/shared/fiber/ReactFiber.js
+20
-18
@@ -82,6 +82,10 @@ export type Fiber = Instance & {
82
// memory if we need to.
83
alternate: ?Fiber,
84
85
+ // Conceptual aliases
86
+ // parent : Instance -> returnFiber The parent happens to be the same as the return fiber.
87
+ // workInProgress : Fiber -> alternate The alternate used for reuse happens to be the same as work in progress.
88
+
89
};
90
91
var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
@@ -128,29 +132,27 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
132
// that we're free to reuse. This is lazily created to avoid allocating extra
133
// objects for things that are never updated. It also allow us to reclaim the
134
// extra memory if needed.
131
- if (fiber.alternate) {
132
- fiber.alternate.pendingWorkPriority = priorityLevel;
133
- return fiber.alternate;
134
- }
135
- // This should not have an alternate already
136
- var alt = createFiber(fiber.tag, fiber.key);
137
-
138
- if (fiber.parent) {
139
- // TODO: This assumes the parent's alternate is already created.
140
- // Stop using the alternates of parents once we have a parent stack.
141
- // $FlowFixMe: This downcast is not safe. It is intentionally an error.
142
- alt.parent = fiber.parent.alternate;
143
- if (!alt.parent) {
144
- // TODO: There needs to be a "work in progress" tree all the way up to
145
- // the final root.
146
- throw new Error('The parent must have an alternate already.');
147
- }
135
+ let alt = fiber.alternate;
136
+ if (alt) {
137
+ alt.stateNode = fiber.stateNode;
138
+ alt.child = fiber.child;
139
+ alt.sibling = fiber.sibling;
140
+ alt.ref = alt.ref;
141
+ alt.pendingProps = fiber.pendingProps;
142
+ alt.pendingWorkPriority = priorityLevel;
143
+ return alt;
144
}
145
146
+ // This should not have an alternate already
147
+ alt = createFiber(fiber.tag, fiber.key);
148
alt.type = fiber.type;
149
alt.stateNode = fiber.stateNode;
152
- alt.alternate = fiber;
150
+ alt.child = fiber.child;
151
+ alt.sibling = fiber.sibling;
152
+ alt.ref = alt.ref;
153
alt.pendingWorkPriority = priorityLevel;
154
+
155
+ alt.alternate = fiber;
156
fiber.alternate = alt;
157
return alt;
158
};
src/renderers/shared/fiber/ReactFiberBeginWork.js
+3
@@ -125,6 +125,9 @@ function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
125
// and children from that node.
126
workInProgress.output = current.output;
127
workInProgress.child = current.child;
128
+ if (workInProgress.child) {
129
+ workInProgress.child.parent = workInProgress;
130
+ }
131
workInProgress.stateNode = current.stateNode;
132
workInProgress.pendingWorkPriority = NoWork;
133
return null;
src/renderers/shared/fiber/ReactFiberScheduler.js
+37
-10
@@ -42,8 +42,21 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
42
let nextScheduledRoot : ?FiberRoot = null;
43
let lastScheduledRoot : ?FiberRoot = null;
44
45
+ function cloneSiblings(current : Fiber, workInProgress : Fiber, parent : Fiber) {
46
+ while (current.sibling) {
47
+ current = current.sibling;
48
+ workInProgress = workInProgress.sibling = ReactFiber.cloneFiber(
49
+ current,
50
+ current.pendingWorkPriority
51
+ );
52
+ workInProgress.parent = parent;
53
+ }
54
+ workInProgress.sibling = null;
55
+ }
56
+
57
function findNextUnitOfWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) : ?Fiber {
58
let current = root.current;
59
+ ReactFiber.cloneFiber(current, current.pendingWorkPriority);
60
while (current) {
61
if (current.pendingWorkPriority !== 0 &&
62
current.pendingWorkPriority <= priorityLevel) {
@@ -51,9 +64,12 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
64
if (current.pendingProps !== null) {
65
// We found some work to do. We need to return the "work in progress"
66
// of this node which will be the alternate.
54
- const clone = ReactFiber.cloneFiber(current, current.pendingWorkPriority);
55
- clone.pendingProps = current.pendingProps;
56
- return clone;
67
+ const workInProgress = current.alternate;
68
+ if (!workInProgress) {
69
+ throw new Error('Should have wip now');
70
+ }
71
+ workInProgress.pendingProps = current.pendingProps;
72
+ return workInProgress;
73
}
74
// If we have a child let's see if any of our children has work to do.
75
// Only bother doing this at all if the current priority level matches
@@ -62,7 +78,15 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
78
// type of child that needs to be searched for work.
79
if (current.child) {
80
// Ensure we have a work in progress copy to backtrack through.
65
- ReactFiber.cloneFiber(current, NoWork);
81
+ let currentChild = current.child;
82
+ let workInProgress = current.alternate;
83
+ if (!workInProgress) {
84
+ throw new Error('Should have wip now');
85
+ }
86
+ workInProgress.pendingWorkPriority = current.pendingWorkPriority;
87
+ workInProgress.child = ReactFiber.cloneFiber(currentChild, NoWork);
88
+ workInProgress.child.parent = workInProgress;
89
+ cloneSiblings(currentChild, workInProgress.child, workInProgress);
90
current = current.child;
91
continue;
92
}
@@ -164,15 +188,18 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
188
// If we're at the root, there's no more work to do. We can flush it.
189
const root : FiberRoot = (workInProgress.stateNode : any);
190
root.current = workInProgress;
167
- console.log('completed one root flush with remaining work at priority', workInProgress.pendingWorkPriority);
168
- const hasMoreWork = workInProgress.pendingWorkPriority !== NoWork;
191
// TODO: We can be smarter here and only look for more work in the
192
// "next" scheduled work since we've already scanned passed. That
193
// also ensures that work scheduled during reconciliation gets deferred.
194
+ // const hasMoreWork = workInProgress.pendingWorkPriority !== NoWork;
195
const nextWork = findNextUnitOfWork();
173
- if (!nextWork && hasMoreWork) {
174
- throw new Error('FiberRoots should not have flagged more work if there is none.');
175
- }
196
+ // if (!nextWork && hasMoreWork) {
197
+ // TODO: This can happen when some deep work completes and we don't
198
+ // know if this was the last one. We should be able to keep track of
199
+ // the highest priority still in the tree for one pass. But if we
200
+ // terminate an update we don't know.
201
+ // throw new Error('FiberRoots should not have flagged more work if there is none.');
202
+ // }
203
return nextWork;
204
}
205
}
@@ -181,7 +208,7 @@ module.exports = function<T, P, I>(config : HostConfig<T, P, I>) {
208
function performUnitOfWork(workInProgress : Fiber) : ?Fiber {
209
// Ignore work if there is nothing to do.
210
if (workInProgress.pendingProps === null) {
184
- return null;
211
+ return completeUnitOfWork(workInProgress);
212
}
213
// The current, flushed, state of this fiber is the alternate.
214
// Ideally nothing should rely on this, but relying on it here
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+7
-6
@@ -182,12 +182,13 @@ describe('ReactIncremental', function() {
182
return (
183
<div>
184
<Bar>{props.text}</Bar>
185
- <div hidden={true}>
186
- <span>
185
+ <content hidden={true}>
186
<Middle>{props.text}</Middle>
188
- </span>
189
- </div>
187
+ </content>
188
<Bar>{props.text}</Bar>
189
+ <footer hidden={true}>
190
+ <Middle>Footer</Middle>
191
+ </footer>
192
</div>
193
);
194
}
@@ -196,7 +197,7 @@ describe('ReactIncremental', function() {
197
ReactNoop.render(<Foo text="foo" />);
198
ReactNoop.flush();
199
199
- expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Middle']);
200
+ expect(ops).toEqual(['Foo', 'Bar', 'Bar', 'Middle', 'Middle']);
201
202
ops = [];
203
@@ -212,7 +213,7 @@ describe('ReactIncremental', function() {
213
// Flush only the remaining work
214
ReactNoop.flush();
215
215
- expect(ops).toEqual(['Middle']);
216
+ expect(ops).toEqual(['Middle', 'Middle']);
217
218
});
219