Reuse the correct child and side-effects when reusing partial work
We need to use the *other* child because we reset it to the current one on the way up. We also need to reset the first/last side-effects to that of the children so that we're committing the right thing.
Sebastian Markbage committed
Jul 18, 2016 at 20:00 UTC
de6069e5504e5b4d71dada5da9a771f11f00c6cc
3 files changed
+89
-4
src/renderers/shared/fiber/ReactFiberBeginWork.js
+28
-1
@@ -148,6 +148,23 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
148
} while (child = child.sibling);
149
}
150
151
+ function reuseChildrenEffects(returnFiber : Fiber, firstChild : Fiber) {
152
+ let child = firstChild;
153
+ do {
154
+ // Ensure that the first and last effect of the parent corresponds
155
+ // to the children's first and last effect.
156
+ if (!returnFiber.firstEffect) {
157
+ returnFiber.firstEffect = child.firstEffect;
158
+ }
159
+ if (child.lastEffect) {
160
+ if (returnFiber.lastEffect) {
161
+ returnFiber.lastEffect.nextEffect = child.firstEffect;
162
+ }
163
+ returnFiber.lastEffect = child.lastEffect;
164
+ }
165
+ } while (child = child.sibling);
166
+ }
167
+
168
function beginWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber {
169
// The current, flushed, state of this fiber is the alternate.
170
// Ideally nothing should rely on this, but relying on it here
@@ -191,7 +208,17 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
208
const priorityLevel = workInProgress.pendingWorkPriority;
209
workInProgress.pendingProps = null;
210
workInProgress.pendingWorkPriority = NoWork;
194
- if (workInProgress.child) {
211
+
212
+ workInProgress.firstEffect = null;
213
+ workInProgress.nextEffect = null;
214
+ workInProgress.lastEffect = null;
215
+
216
+ if (workInProgress.child && workInProgress.child.alternate) {
217
+ // On the way up here, we reset the child node to be the current one.
218
+ // Therefore we have to reuse the alternate. This is super weird.
219
+ workInProgress.child = workInProgress.child.alternate;
220
+ // Ensure that the effects of reused work are preserved.
221
+ reuseChildrenEffects(workInProgress, workInProgress.child);
222
// If we bail out but still has work with the current priority in this
223
// subtree, we need to go find it right now. If we don't, we won't flush
224
// it until the next tick.
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
-2
@@ -394,8 +394,6 @@ describe('ReactIncremental', function() {
394
);
395
}
396
397
- // Start rendering an update
398
-
397
// Init
398
ReactNoop.render(<Foo text="foo" step={0} />);
399
ReactNoop.flush();
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+61
-1
@@ -14,7 +14,7 @@
14
var React;
15
var ReactNoop;
16
17
-describe('ReactIncremental', function() {
17
+describe('ReactIncrementalSideEffects', function() {
18
beforeEach(function() {
19
React = require('React');
20
ReactNoop = require('ReactNoop');
@@ -127,6 +127,66 @@ describe('ReactIncremental', function() {
127
128
});
129
130
+ it('can reuse side-effects after being preempted', function() {
131
+
132
+ function Bar(props) {
133
+ return <span prop={props.children} />;
134
+ }
135
+
136
+ var middleContent = (
137
+ <div>
138
+ <Bar>Hello</Bar>
139
+ <Bar>World</Bar>
140
+ </div>
141
+ );
142
+
143
+ function Foo(props) {
144
+ return (
145
+ <div hidden={true}>
146
+ {
147
+ props.step === 0 ?
148
+ <div>
149
+ <Bar>Hi</Bar>
150
+ <Bar>{props.text}</Bar>
151
+ </div>
152
+ : middleContent
153
+ }
154
+ </div>
155
+ );
156
+ }
157
+
158
+ // Init
159
+ ReactNoop.render(<Foo text="foo" step={0} />);
160
+ ReactNoop.flush();
161
+
162
+ expect(ReactNoop.root.children).toEqual([
163
+ div(div(span('Hi'), span('foo'))),
164
+ ]);
165
+
166
+ // Make a quick update which will schedule low priority work to
167
+ // update the middle content.
168
+ ReactNoop.render(<Foo text="bar" step={1} />);
169
+ ReactNoop.flushLowPri(30);
170
+
171
+ // The tree remains unchanged.
172
+ expect(ReactNoop.root.children).toEqual([
173
+ div(div(span('Hi'), span('foo'))),
174
+ ]);
175
+
176
+ // The first Bar has already completed its update but we'll interupt it to
177
+ // render some higher priority work. The middle content will bailout so
178
+ // it remains untouched which means that it should reuse it next time.
179
+ ReactNoop.render(<Foo text="foo" step={1} />);
180
+ ReactNoop.flush(30);
181
+
182
+ // Since we did nothing to the middle subtree during the interuption,
183
+ // we should be able to reuse the reconciliation work that we already did
184
+ // without restarting. The side-effects should still be replayed.
185
+
186
+ expect(ReactNoop.root.children).toEqual([
187
+ div(div(span('Hello'), span('World'))),
188
+ ]);
189
+ });
190
191
it('updates a child even though the old props is empty', function() {
192
function Foo(props) {