When a reconciliation gets down prioritized, reuse children
When we downprioritize children we need to remember to reuse the old children in the update side-effect. This whole set up is very unfortunate since we can have children in our active tree that never actually finished rendering. This strategy might be fundamentally flawed, not sure.
Sebastian Markbage committed
Jul 18, 2016 at 16:22 UTC
5ac87e0d0fe3227ec64bd1fec55aa6bb5918026d
5 files changed
+64
-3
src/renderers/shared/fiber/ReactFiber.js
+6
@@ -106,6 +106,11 @@ export type Fiber = Instance & {
106
// I'm not really sure so I'll use a flag for now.
107
// TODO: Find another way to infer this flag.
108
hasWorkInProgress: bool,
109
+ // Keeps track if we've deprioritized the children of this node so we know to
110
+ // reuse the existing children.
111
+ // TODO: Find another way to infer this flag or separate children updates from
112
+ // property updates so that we simply don't try to update children here.
113
+ wasDeprioritized: bool,
114
115
// Conceptual aliases
116
// workInProgress : Fiber -> alternate The alternate used for reuse happens
@@ -146,6 +151,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
151
pendingWorkPriority: NoWork,
152
153
hasWorkInProgress: false,
154
+ wasDeprioritized: false,
155
156
alternate: null,
157
src/renderers/shared/fiber/ReactFiberBeginWork.js
+2
@@ -82,6 +82,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
82
OffscreenPriority
83
);
84
workInProgress.pendingWorkPriority = OffscreenPriority;
85
+ workInProgress.wasDeprioritized = true;
86
return null;
87
} else {
88
workInProgress.child = reconcileChildFibers(
@@ -91,6 +92,7 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
92
priority
93
);
94
workInProgress.pendingWorkPriority = NoWork;
95
+ workInProgress.wasDeprioritized = false;
96
return workInProgress.child;
97
}
98
}
src/renderers/shared/fiber/ReactFiberCommitWork.js
+8
-1
@@ -47,7 +47,14 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
47
throw new Error('This should only be done during updates.');
48
}
49
// Commit the work prepared earlier.
50
- const child = (finishedWork.child : ?Fiber);
50
+ let child;
51
+ if (finishedWork.wasDeprioritized) {
52
+ // If this was a down priority, we need to preserve the old child in
53
+ // the output.
54
+ child = finishedWork.alternate ? finishedWork.alternate.child : null;
55
+ } else {
56
+ child = finishedWork.child;
57
+ }
58
const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child;
59
const newProps = finishedWork.memoizedProps;
60
const current = finishedWork.alternate;
src/renderers/shared/fiber/ReactFiberCompleteWork.js
+9
-2
@@ -140,9 +140,16 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
140
markForPreEffect(workInProgress);
141
return null;
142
case HostComponent:
143
- const child = workInProgress.child;
144
- const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child;
143
+ let child;
144
let newProps = workInProgress.pendingProps;
145
+ if (workInProgress.wasDeprioritized) {
146
+ // If this was a down priority, we need to preserve the old child in
147
+ // the output.
148
+ child = current ? current.child : null;
149
+ } else {
150
+ child = workInProgress.child;
151
+ }
152
+ const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child;
153
if (current && workInProgress.stateNode != null) {
154
// If we have an alternate, that means this is an update and we need to
155
// schedule a side-effect to do the updates.
src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
+39
@@ -89,6 +89,45 @@ describe('ReactIncremental', function() {
89
90
});
91
92
+ it('preserves a previously rendered node when deprioritized', function() {
93
+
94
+ function Middle(props) {
95
+ return <span prop={props.children} />;
96
+ }
97
+
98
+ function Foo(props) {
99
+ return (
100
+ <div>
101
+ <div hidden={true}>
102
+ <Middle>{props.text}</Middle>
103
+ </div>
104
+ </div>
105
+ );
106
+ }
107
+
108
+ ReactNoop.render(<Foo text="foo" />);
109
+ ReactNoop.flush();
110
+
111
+ expect(ReactNoop.root.children).toEqual([
112
+ div(div(span('foo'))),
113
+ ]);
114
+
115
+ ReactNoop.render(<Foo text="bar" />);
116
+ ReactNoop.flushLowPri(20);
117
+
118
+ expect(ReactNoop.root.children).toEqual([
119
+ div(div(span('foo'))),
120
+ ]);
121
+
122
+ ReactNoop.flush();
123
+
124
+ expect(ReactNoop.root.children).toEqual([
125
+ div(div(span('bar'))),
126
+ ]);
127
+
128
+ });
129
+
130
+
131
it('updates a child even though the old props is empty', function() {
132
function Foo(props) {
133
return (