New effect type: Callback

This solves the problem I had with enqueueSetState and enqueueCallback
being separate.
This commit is contained in:
Andrew Clark
2016-11-02 12:40:21 -07:00
parent 6041ebcc82
commit bea009255e
13 changed files with 169 additions and 116 deletions
+3 -3
View File
@@ -87,6 +87,9 @@ src/renderers/art/__tests__/ReactART-test.js
* resolves refs before componentDidMount
* resolves refs before componentDidUpdate
src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should throw with an error code in production
src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
* should set style attribute when styles exist
* should warn when using hyphenated style names
@@ -369,7 +372,6 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js
* should account for escaping on a checksum mismatch
* should warn if render removes React-rendered children
* should warn if the unmounted node was rendered by another copy of React
* passes the correct callback context
* tracks root instances
* marks top-level mounts
@@ -608,8 +610,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
* throws in setState if the update callback is not a function
* throws in replaceState if the update callback is not a function
* throws in forceUpdate if the update callback is not a function
* does not update one component twice in a batch (#2410)
* unstable_batchedUpdates should return value from a callback
src/renderers/shared/stack/reconciler/__tests__/refs-test.js
* Should increase refs with an increase in divs
+6 -1
View File
@@ -438,7 +438,6 @@ src/renderers/dom/__tests__/ReactDOMProduction-test.js
* should use prod React
* should handle a simple flow
* should call lifecycle methods
* should throw with an error code in production
src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render strings as children
@@ -692,6 +691,7 @@ src/renderers/dom/stack/client/__tests__/ReactMount-test.js
* should warn if mounting into dirty rendered markup
* should not warn if mounting into non-empty node
* should warn when mounting into document.body
* passes the correct callback context
src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js
* should destroy a react root upon request
@@ -788,6 +788,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* calls componentWill* twice if an update render is aborted
* does not call componentWillReceiveProps for state-only updates
* skips will/DidUpdate when bailing unless an update was already in progress
* performs batched updates at the end of the batch
* can nest batchedUpdates
src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js
* handles isMounted even when the initial render is deferred
@@ -833,6 +835,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalSideEffects-test.js
* can defer side-effects and reuse them later - complex
* deprioritizes setStates that happens within a deprioritized tree
* calls callback after update is flushed
* calls setState callback even if component bails out
* calls componentWillUnmount after a deletion, even if nested
* calls componentDidMount/Update after insertion/update
* invokes ref callbacks after insertion/update/unmount
@@ -1124,7 +1127,9 @@ src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js
* should not reconcile children passed via props
* should queue nested updates
* calls componentWillReceiveProps setState callback properly
* does not update one component twice in a batch (#2410)
* does not update one component twice in a batch (#6371)
* unstable_batchedUpdates should return value from a callback
src/renderers/shared/stack/reconciler/__tests__/Transaction-test.js
* should invoke closers with/only-with init returns
@@ -97,10 +97,10 @@ describe('ReactTransitionGroup', () => {
expect(log).toEqual(['didMount', 'willEnter', 'didEnter']);
log = [];
instance.setState({count: 1}, function() {
expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']);
});
instance.setState({count: 1});
});
expect(log).toEqual(['willLeave', 'didLeave', 'willUnmount']);
});
it('should handle enter/leave/enter/leave correctly', () => {
@@ -728,11 +728,6 @@ var ReactClassMixin = {
* type signature and the only use case for this, is to avoid that.
*/
replaceState: function(newState, callback) {
if (this.updater.isFiberUpdater) {
this.updater.enqueueReplaceState(this, newState, callback);
return;
}
this.updater.enqueueReplaceState(this, newState);
if (callback) {
this.updater.enqueueCallback(this, callback, 'replaceState');
@@ -65,12 +65,6 @@ ReactComponent.prototype.setState = function(partialState, callback) {
'setState(...): takes an object of state variables to update or a ' +
'function which returns an object of state variables.'
);
if (this.updater.isFiberUpdater) {
this.updater.enqueueSetState(this, partialState, callback);
return;
}
this.updater.enqueueSetState(this, partialState);
if (callback) {
this.updater.enqueueCallback(this, callback, 'setState');
@@ -92,11 +86,6 @@ ReactComponent.prototype.setState = function(partialState, callback) {
* @protected
*/
ReactComponent.prototype.forceUpdate = function(callback) {
if (this.updater.isFiberUpdater) {
this.updater.enqueueForceUpdate(this, callback);
return;
}
this.updater.enqueueForceUpdate(this);
if (callback) {
this.updater.enqueueCallback(this, callback, 'forceUpdate');
@@ -48,7 +48,7 @@ var ReactFiberClassComponent = require('ReactFiberClassComponent');
module.exports = function<T, P, I, TI, C>(
config : HostConfig<T, P, I, TI, C>,
scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void
scheduleUpdate : (fiber: Fiber) => void
) {
const {
@@ -26,7 +26,7 @@ var { isMounted } = require('ReactFiberTreeReflection');
var ReactInstanceMap = require('ReactInstanceMap');
var shallowEqual = require('shallowEqual');
module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?PriorityLevel) => void) {
module.exports = function(scheduleUpdate : (fiber: Fiber) => void) {
function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) {
fiber.updateQueue = updateQueue;
@@ -41,35 +41,33 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
// Class component state updater
const updater = {
isMounted,
enqueueSetState(instance, partialState, callback) {
enqueueSetState(instance, partialState) {
const fiber = ReactInstanceMap.get(instance);
const updateQueue = fiber.updateQueue ?
addToQueue(fiber.updateQueue, partialState) :
createUpdateQueue(partialState);
if (callback) {
addCallbackToQueue(updateQueue, callback);
}
scheduleUpdateQueue(fiber, updateQueue);
},
enqueueReplaceState(instance, state, callback) {
enqueueReplaceState(instance, state) {
const fiber = ReactInstanceMap.get(instance);
const updateQueue = createUpdateQueue(state);
updateQueue.isReplace = true;
if (callback) {
addCallbackToQueue(updateQueue, callback);
}
scheduleUpdateQueue(fiber, updateQueue);
},
enqueueForceUpdate(instance, callback) {
enqueueForceUpdate(instance) {
const fiber = ReactInstanceMap.get(instance);
const updateQueue = fiber.updateQueue || createUpdateQueue(null);
updateQueue.isForced = true;
if (callback) {
addCallbackToQueue(updateQueue, callback);
}
scheduleUpdateQueue(fiber, updateQueue);
},
isFiberUpdater: true,
enqueueCallback(instance, callback) {
const fiber = ReactInstanceMap.get(instance);
let updateQueue = fiber.updateQueue ?
fiber.updateQueue :
createUpdateQueue(null);
addCallbackToQueue(updateQueue, callback);
scheduleUpdateQueue(fiber, updateQueue);
},
};
function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) {
@@ -210,11 +208,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : ?Prior
// TODO: Previous state can be null.
let newState;
if (updateQueue) {
newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps);
if (!updateQueue.hasUpdate) {
newState = previousState;
} else {
newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps);
}
} else {
newState = previousState;
}
if (oldProps === newProps &&
previousState === newState &&
updateQueue && !updateQueue.isForced) {
return false;
}
if (!checkShouldComponentUpdate(
workInProgress,
oldProps,
@@ -29,7 +29,8 @@ var { callCallbacks } = require('ReactFiberUpdateQueue');
var {
Placement,
PlacementAndUpdate,
Update,
Callback,
} = require('ReactTypeOfSideEffect');
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
@@ -102,8 +103,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
// If it is not host node and, we might have a host node inside it.
// Try to search down until we find one.
// TODO: For coroutines, this will have to search the stateNode.
if (node.effectTag === Placement ||
node.effectTag === PlacementAndUpdate) {
if (node.effectTag & Placement) {
// If we don't have a child, try the siblings instead.
continue siblings;
}
@@ -114,8 +114,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}
// Check if this host node is stable or about to be placed.
if (node.effectTag !== Placement &&
node.effectTag !== PlacementAndUpdate) {
if (!(node.effectTag & Placement)) {
// Found it!
return node.stateNode;
}
@@ -329,41 +328,41 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
case ClassComponent: {
const instance = finishedWork.stateNode;
let error = null;
if (!current) {
if (typeof instance.componentDidMount === 'function') {
error = tryCallComponentDidMount(instance);
}
} else {
if (typeof instance.componentDidUpdate === 'function') {
const prevProps = current.memoizedProps;
const prevState = current.memoizedState;
error = tryCallComponentDidUpdate(instance, prevProps, prevState);
if (finishedWork.effectTag & Update) {
if (!current) {
if (typeof instance.componentDidMount === 'function') {
error = tryCallComponentDidMount(instance);
}
} else {
if (typeof instance.componentDidUpdate === 'function') {
const prevProps = current.memoizedProps;
const prevState = current.memoizedState;
error = tryCallComponentDidUpdate(instance, prevProps, prevState);
}
}
attachRef(current, finishedWork, instance);
}
// Clear updates from current fiber. This must go before the callbacks
// are reset, in case an update is triggered from inside a callback. Is
// this safe? Relies on the assumption that work is only committed if
// the update queue is empty.
// Clear updates from current fiber.
if (finishedWork.alternate) {
finishedWork.alternate.updateQueue = null;
}
if (finishedWork.callbackList) {
const { callbackList } = finishedWork;
finishedWork.callbackList = null;
callCallbacks(callbackList, instance);
if (finishedWork.effectTag & Callback) {
if (finishedWork.callbackList) {
callCallbacks(finishedWork.callbackList, instance);
finishedWork.callbackList = null;
}
}
attachRef(current, finishedWork, instance);
if (error) {
return trapError(finishedWork, error);
}
return null;
}
case HostContainer: {
const instance = finishedWork.stateNode;
if (instance.callbackList) {
const { callbackList } = instance;
instance.callbackList = null;
callCallbacks(callbackList, instance.current.child.stateNode);
const rootFiber = finishedWork.stateNode;
if (rootFiber.callbackList) {
const { callbackList } = rootFiber;
rootFiber.callbackList = null;
callCallbacks(callbackList, rootFiber.current.child.stateNode);
}
}
case HostComponent: {
@@ -34,6 +34,7 @@ var {
} = ReactTypeOfWork;
var {
Update,
Callback,
} = ReactTypeOfSideEffect;
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
@@ -48,6 +49,11 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
workInProgress.effectTag |= Update;
}
function markCallback(workInProgress : Fiber) {
// Tag the fiber with a callback effect.
workInProgress.effectTag |= Callback;
}
function transferOutput(child : ?Fiber, returnFiber : Fiber) {
// If we have a single result, we just pass that through as the output to
// avoid unnecessary traversal. When we have multiple output, we just pass
@@ -124,19 +130,24 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
// Also need to transfer the props, because pendingProps will be null
// in the case of an update
const { state, props } = workInProgress.stateNode;
const updateQueue = workInProgress.updateQueue;
workInProgress.memoizedState = state;
workInProgress.memoizedProps = props;
// Transfer update queue to callbackList field so callbacks can be
// called during commit phase.
workInProgress.callbackList = workInProgress.updateQueue;
if (current) {
if (current.memoizedProps !== workInProgress.memoizedProps ||
current.memoizedState !== workInProgress.memoizedState) {
current.memoizedState !== workInProgress.memoizedState ||
updateQueue && updateQueue.isForced) {
markUpdate(workInProgress);
}
} else {
markUpdate(workInProgress);
}
if (updateQueue && updateQueue.hasCallback) {
// Transfer update queue to callbackList field so callbacks can be
// called during commit phase.
workInProgress.callbackList = updateQueue;
markCallback(workInProgress);
}
return null;
case HostContainer:
transferOutput(workInProgress.child, workInProgress);
@@ -39,6 +39,11 @@ var {
Update,
PlacementAndUpdate,
Deletion,
Callback,
PlacementAndCallback,
UpdateAndCallback,
PlacementAndUpdateAndCallback,
DeletionAndCallback,
} = require('ReactTypeOfSideEffect');
var {
@@ -52,9 +57,6 @@ if (__DEV__) {
var timeHeuristicForUnitOfWork = 1;
module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
// Use a closure to circumvent the circular dependency between the scheduler
// and ReactFiberBeginWork. Don't know if there's a better way to do this.
const { beginWork } = ReactFiberBeginWork(config, scheduleUpdate);
const { completeWork } = ReactFiberCompleteWork(config);
const { commitInsertion, commitDeletion, commitWork, commitLifeCycles } =
@@ -139,28 +141,34 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
let effectfulFiber = finishedWork.firstEffect;
while (effectfulFiber) {
switch (effectfulFiber.effectTag) {
case Placement: {
case Placement:
case PlacementAndCallback: {
commitInsertion(effectfulFiber);
// Clear the effect tag so that we know that this is inserted, before
// Clear the "placement" from effect tag so that we know that this is inserted, before
// any life-cycles like componentDidMount gets called.
effectfulFiber.effectTag = NoEffect;
break;
}
case PlacementAndUpdate: {
case PlacementAndUpdate:
case PlacementAndUpdateAndCallback: {
// Placement
commitInsertion(effectfulFiber);
const current = effectfulFiber.alternate;
commitWork(current, effectfulFiber);
// Clear the "placement" from effect tag so that we know that this is inserted, before
// any life-cycles like componentDidMount gets called.
effectfulFiber.effectTag = Update;
break;
}
case Update: {
// Update
const current = effectfulFiber.alternate;
commitWork(current, effectfulFiber);
break;
}
case Deletion: {
case Update:
case UpdateAndCallback:
const current = effectfulFiber.alternate;
commitWork(current, effectfulFiber);
break;
case Deletion:
case DeletionAndCallback:
// Deletion might cause an error in componentWillUnmount().
// We will continue nevertheless and handle those later on.
const trappedErrors = commitDeletion(effectfulFiber);
@@ -176,8 +184,8 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}
break;
}
}
effectfulFiber = effectfulFiber.nextEffect;
}
@@ -186,8 +194,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
// already been invoked.
effectfulFiber = finishedWork.firstEffect;
while (effectfulFiber) {
if (effectfulFiber.effectTag === Update ||
effectfulFiber.effectTag === PlacementAndUpdate) {
if (effectfulFiber.effectTag & (Update | Callback)) {
const current = effectfulFiber.alternate;
const trappedError = commitLifeCycles(current, effectfulFiber);
if (trappedError) {
@@ -540,22 +547,22 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}
function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : ?Deadline) {
function performAndHandleErrors(priorityLevel : PriorityLevel, deadline : null | Deadline) {
// The exact priority level doesn't matter, so long as it's in range of the
// work (sync, animation, deferred) being performed.
try {
if (priorityLevel === SynchronousPriority) {
performSynchronousWorkUnsafe();
}
if (priorityLevel > AnimationPriority) {
} else if (priorityLevel > AnimationPriority) {
if (!deadline) {
throw new Error('No deadline');
} else {
performDeferredWorkUnsafe(deadline);
}
return;
} else {
performAnimationWorkUnsafe();
}
performAnimationWorkUnsafe();
} catch (error) {
const failedUnitOfWork = nextUnitOfWork;
// Reset because it points to the error boundary:
@@ -639,31 +646,25 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
}
}
function scheduleWork(root : FiberRoot, priorityLevel : ?PriorityLevel) {
if (priorityLevel == null) {
priorityLevel = priorityContext;
}
if (priorityLevel === SynchronousPriority) {
scheduleSynchronousWork(root);
}
function scheduleWork(root : FiberRoot) {
scheduleWorkAtPriority(root, priorityContext);
}
function scheduleWorkAtPriority(root : FiberRoot, priorityLevel : PriorityLevel) {
if (priorityLevel === NoWork) {
return;
}
if (priorityLevel > AnimationPriority) {
} else if (priorityLevel === SynchronousPriority) {
scheduleSynchronousWork(root);
} else if (priorityLevel <= AnimationPriority) {
scheduleAnimationWork(root, priorityLevel);
} else {
scheduleDeferredWork(root, priorityLevel);
return;
}
scheduleAnimationWork(root, priorityLevel);
}
function scheduleUpdate(fiber: Fiber, priorityLevel : ?PriorityLevel): void {
// Use priority context if no priority is provided
if (priorityLevel == null) {
priorityLevel = priorityContext;
}
function scheduleUpdate(fiber : Fiber) {
const priorityLevel = priorityContext;
while (true) {
if (fiber.pendingWorkPriority === NoWork ||
fiber.pendingWorkPriority >= priorityLevel) {
@@ -678,7 +679,7 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
if (!fiber.return) {
if (fiber.tag === HostContainer) {
const root : FiberRoot = (fiber.stateNode : any);
scheduleWork(root, priorityLevel);
scheduleWorkAtPriority(root, priorityLevel);
return;
} else {
throw new Error('Invalid root');
@@ -22,6 +22,8 @@ type UpdateQueueNode = {
export type UpdateQueue = UpdateQueueNode & {
isReplace: boolean,
isForced: boolean,
hasUpdate: boolean,
hasCallback: boolean,
tail: UpdateQueueNode
};
@@ -33,6 +35,8 @@ exports.createUpdateQueue = function(partialState : mixed) : UpdateQueue {
next: null,
isReplace: false,
isForced: false,
hasUpdate: partialState != null,
hasCallback: false,
tail: (null : any),
};
queue.tail = queue;
@@ -48,6 +52,7 @@ function addToQueue(queue : UpdateQueue, partialState : mixed) : UpdateQueue {
};
queue.tail.next = node;
queue.tail = node;
queue.hasUpdate = queue.hasUpdate || (partialState == null);
return queue;
}
@@ -59,15 +64,21 @@ exports.addCallbackToQueue = function(queue : UpdateQueue, callback: Function) :
addToQueue(queue, null);
}
queue.tail.callback = callback;
queue.hasCallback = true;
return queue;
};
exports.callCallbacks = function(queue : UpdateQueue, context : any) {
let node : ?UpdateQueueNode = queue;
while (node) {
if (node.callback && !node.callbackWasCalled) {
const callback = node.callback;
if (callback && !node.callbackWasCalled) {
node.callbackWasCalled = true;
node.callback.call(context);
if (typeof context !== 'undefined') {
callback.call(context);
} else {
callback();
}
}
node = node.next;
}
@@ -12,12 +12,17 @@
'use strict';
export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4;
export type TypeOfSideEffect = 0 | 1 | 2 | 3 | 4 | 8 | 9 | 10 | 11 | 12;
module.exports = {
NoEffect: 0,
Placement: 1,
Update: 2,
PlacementAndUpdate: 3,
Deletion: 4,
NoEffect: 0, // 0b0000
Placement: 1, // 0b0001
Update: 2, // 0b0010
PlacementAndUpdate: 3, // 0b0011
Deletion: 4, // 0b0100
Callback: 8, // 0b1000
PlacementAndCallback: 9, // 0b1001
UpdateAndCallback: 10, // 0b1010
PlacementAndUpdateAndCallback: 11, // 0b1011
DeletionAndCallback: 12, // 0b1100
};
@@ -860,6 +860,35 @@ describe('ReactIncrementalSideEffects', () => {
expect(called).toBe(true);
});
it('calls setState callback even if component bails out', () => {
let instance;
class Foo extends React.Component {
constructor() {
super();
instance = this;
this.state = { text: 'foo' };
}
shouldComponentUpdate(nextProps, nextState) {
return this.state.text !== nextState.text;
}
render() {
return <span prop={this.state.text} />;
}
}
ReactNoop.render(<Foo />);
ReactNoop.flush();
expect(ReactNoop.getChildren()).toEqual([
span('foo'),
]);
let called = false;
instance.setState({}, () => {
called = true;
});
ReactNoop.flush();
expect(called).toBe(true);
});
// TODO: Test that callbacks are not lost if an update is preempted.
it('calls componentWillUnmount after a deletion, even if nested', () => {