Skip to content

Commit 47b386a

Browse files
committed
Update callbacks
Callbacks are stored on the same queue as updates. They care called during the commit phase, after the updates have been flushed. Because the update queue is cleared during the completion phase (before commit), a new field has been added to fiber called callbackList. The queue is transferred from updateQueue to callbackList during completion. During commit, the list is reset. Need a test to confirm that callbacks are not lost if an update is preempted.
1 parent f167826 commit 47b386a

File tree

6 files changed

+98
-7
lines changed

6 files changed

+98
-7
lines changed

src/renderers/shared/fiber/ReactFiber.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ export type Fiber = Instance & {
8383
updateQueue: ?UpdateQueue,
8484
// The state used to create the output. This is a full state object.
8585
memoizedState: any,
86+
// Linked list of callbacks to call after updates are committed.
87+
callbackList: ?UpdateQueue,
8688
// Output is the return value of this fiber, or a linked list of return values
8789
// if this returns multiple values. Such as a fragment.
8890
output: any, // This type will be more specific once we overload the tag.
@@ -141,6 +143,7 @@ var createFiber = function(tag : TypeOfWork, key : null | string) : Fiber {
141143
memoizedProps: null,
142144
updateQueue: null,
143145
memoizedState: null,
146+
callbackList: null,
144147
output: null,
145148

146149
nextEffect: null,
@@ -176,6 +179,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
176179
alt.ref = alt.ref;
177180
alt.pendingProps = fiber.pendingProps;
178181
alt.updateQueue = fiber.updateQueue;
182+
alt.callbackList = fiber.callbackList;
179183
alt.pendingWorkPriority = priorityLevel;
180184

181185
// Whenever we clone, we do so to get a new work in progress.
@@ -198,6 +202,7 @@ exports.cloneFiber = function(fiber : Fiber, priorityLevel : PriorityLevel) : Fi
198202
// pendingProps is here for symmetry but is unnecessary in practice for now.
199203
alt.pendingProps = fiber.pendingProps;
200204
alt.updateQueue = fiber.updateQueue;
205+
alt.callbackList = fiber.callbackList;
201206
alt.pendingWorkPriority = priorityLevel;
202207

203208
alt.alternate = fiber;

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ var { findNextUnitOfWorkAtPriority } = require('ReactFiberPendingWork');
4444
var {
4545
createUpdateQueue,
4646
addToQueue,
47+
addCallbackToQueue,
48+
concatQueues,
4749
mergeUpdateQueue,
4850
} = require('ReactFiberUpdateQueue');
4951
var ReactInstanceMap = require('ReactInstanceMap');
@@ -132,6 +134,29 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>, getSchedu
132134
createUpdateQueue(partialState);
133135
scheduleUpdate(fiber, updateQueue, LowPriority);
134136
},
137+
enqueueCallback(instance, callback) {
138+
const fiber = ReactInstanceMap.get(instance);
139+
let updateQueue = fiber.updateQueue ?
140+
fiber.updateQueue :
141+
createUpdateQueue(null);
142+
addCallbackToQueue(updateQueue, callback);
143+
144+
if (fiber.callbackList) {
145+
// If this fiber was preempted and already has callbacks queued up,
146+
// we need to make sure they are part of the new update queue
147+
updateQueue = concatQueues(fiber.callbackList, updateQueue);
148+
// Safe to reset this now
149+
fiber.callbackList = null;
150+
if (fiber.alternate) {
151+
fiber.alternate.callbackList = null;
152+
}
153+
}
154+
155+
fiber.updateQueue = updateQueue;
156+
if (fiber.alternate) {
157+
fiber.alternate.updateQueue = updateQueue;
158+
}
159+
},
135160
};
136161

137162
function updateClassComponent(current : ?Fiber, workInProgress : Fiber) {

src/renderers/shared/fiber/ReactFiberCommitWork.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ var {
2222
HostContainer,
2323
HostComponent,
2424
} = ReactTypeOfWork;
25+
var { callCallbacks } = require('ReactFiberUpdateQueue');
2526

2627
module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
2728

@@ -31,6 +32,13 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
3132
function commitWork(finishedWork : Fiber) : void {
3233
switch (finishedWork.tag) {
3334
case ClassComponent: {
35+
if (finishedWork.callbackList) {
36+
callCallbacks(finishedWork.callbackList);
37+
finishedWork.callbackList = null;
38+
if (finishedWork.alternate) {
39+
finishedWork.alternate.callbackList = null;
40+
}
41+
}
3442
// TODO: Fire componentDidMount/componentDidUpdate, update refs
3543
return;
3644
}

src/renderers/shared/fiber/ReactFiberCompleteWork.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
4646
}
4747
}
4848

49-
/*
5049
// TODO: It's possible this will create layout thrash issues because mutations
5150
// of the DOM and life-cycles are interleaved. E.g. if a componentDidMount
5251
// of a sibling reads, then the next sibling updates and reads etc.
@@ -59,7 +58,6 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
5958
}
6059
workInProgress.lastEffect = workInProgress;
6160
}
62-
*/
6361

6462
function transferOutput(child : ?Fiber, returnFiber : Fiber) {
6563
// If we have a single result, we just pass that through as the output to
@@ -133,9 +131,16 @@ module.exports = function<T, P, I, C>(config : HostConfig<T, P, I, C>) {
133131
case ClassComponent:
134132
transferOutput(workInProgress.child, workInProgress);
135133
// Don't use the state queue to compute the memoized state. We already
136-
// merged it and assigned it to the instance. Copy it from there.
134+
// merged it and assigned it to the instance. Transfer it from there.
137135
const state = workInProgress.stateNode.state;
138136
workInProgress.memoizedState = state;
137+
// Transfer update queue to callbackList field so callbacks can be
138+
// called during commit phase.
139+
workInProgress.callbackList = workInProgress.updateQueue;
140+
if (current) {
141+
current.callbackList = workInProgress.callbackList;
142+
}
143+
markForPostEffect(workInProgress);
139144
return null;
140145
case HostContainer:
141146
transferOutput(workInProgress.child, workInProgress);

src/renderers/shared/fiber/ReactFiberUpdateQueue.js

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
type UpdateQueueNode = {
1616
partialState: any,
1717
callback: ?Function,
18+
callbackWasCalled: boolean,
1819
next: ?UpdateQueueNode,
1920
};
2021

@@ -26,6 +27,7 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue {
2627
const queue = {
2728
partialState,
2829
callback: null,
30+
callbackWasCalled: false,
2931
next: null,
3032
tail: (null : any),
3133
};
@@ -37,19 +39,29 @@ exports.addToQueue = function(queue : UpdateQueue, partialState : mixed) : Updat
3739
const node = {
3840
partialState,
3941
callback: null,
42+
callbackWasCalled: false,
4043
next: null,
4144
};
4245
queue.tail.next = node;
4346
queue.tail = node;
4447
return queue;
4548
};
4649

47-
exports.callCallbacks = function(queue : UpdateQueue, partialState : mixed) {
50+
exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) : UpdateQueue {
51+
if (queue.tail.callback) {
52+
// If the tail already as a callback, add an empty node to queue
53+
exports.addToQueue(queue, null);
54+
}
55+
queue.tail.callback = callback;
56+
return queue;
57+
};
58+
59+
exports.callCallbacks = function(queue : UpdateQueue, context : any) {
4860
let node : ?UpdateQueueNode = queue;
4961
while (node) {
50-
if (node.callback) {
51-
const { callback } = node;
52-
callback();
62+
if (node.callback && !node.callbackWasCalled) {
63+
node.callbackWasCalled = true;
64+
node.callback.call(context);
5365
}
5466
node = node.next;
5567
}
@@ -71,3 +83,9 @@ exports.mergeUpdateQueue = function(queue : UpdateQueue, prevState : any, props
7183
}
7284
return state;
7385
};
86+
87+
exports.concatQueues = function(a: UpdateQueue, b: UpdateQueue): UpdateQueue {
88+
a.tail.next = b;
89+
a.tail = b;
90+
return a;
91+
}

src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,4 +274,34 @@ describe('ReactIncrementalSideEffects', function() {
274274
// moves to "current" without flushing due to having lower priority. Does this
275275
// even happen? Maybe a child doesn't get processed because it is lower prio?
276276

277+
it('calls callback after update is flushed', () => {
278+
let instance;
279+
class Foo extends React.Component {
280+
constructor() {
281+
super();
282+
instance = this;
283+
this.state = { text: 'foo' };
284+
}
285+
render() {
286+
return <span prop={this.state.text} />;
287+
}
288+
}
289+
290+
ReactNoop.render(<Foo />);
291+
ReactNoop.flush();
292+
expect(ReactNoop.root.children).toEqual([
293+
span('foo'),
294+
]);
295+
let called = false;
296+
instance.setState({ text: 'bar' }, () => {
297+
expect(ReactNoop.root.children).toEqual([
298+
span('bar'),
299+
]);
300+
called = true;
301+
});
302+
ReactNoop.flush();
303+
expect(called).toBe(true);
304+
});
305+
306+
// TODO: Test that callbacks are not lost if an update is preempted.
277307
});

0 commit comments

Comments
 (0)