Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Use synchronous scheduling by default #8127

Merged
merged 14 commits into from
Nov 2, 2016
Merged
3 changes: 0 additions & 3 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,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

Expand Down Expand Up @@ -608,8 +607,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
Expand Down
6 changes: 6 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just learned about this a week ago. This is the weirdest part of React API 😄


src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js
* should destroy a react root upon request
Expand Down Expand Up @@ -788,6 +789,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
Expand Down Expand Up @@ -833,6 +836,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
Expand Down Expand Up @@ -1124,7 +1128,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
Expand Down
6 changes: 3 additions & 3 deletions src/addons/transitions/__tests__/ReactTransitionGroup-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
6 changes: 6 additions & 0 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ var DOMRenderer = ReactFiberReconciler({

scheduleDeferredCallback: window.requestIdleCallback,

useSyncScheduling: true,

});

var warned = false;
Expand Down Expand Up @@ -163,6 +165,10 @@ var ReactDOM = {
return DOMRenderer.findHostInstance(component);
},

unstable_batchedUpdates<A>(fn : () => A) : A {
return DOMRenderer.batchedUpdates(fn);
},

};

module.exports = ReactDOM;
4 changes: 4 additions & 0 deletions src/renderers/noop/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,10 @@ var ReactNoop = {
NoopRenderer.performWithPriority(AnimationPriority, fn);
},

batchedUpdates: NoopRenderer.batchedUpdates,

syncUpdates: NoopRenderer.syncUpdates,

// Logs the current state of the tree.
dumpTree(rootID : string = DEFAULT_ROOT_ID) {
const root = roots.get(rootID);
Expand Down
4 changes: 2 additions & 2 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import type { ReactCoroutine } from 'ReactCoroutine';
import type { Fiber } from 'ReactFiber';
import type { HostConfig } from 'ReactFiberReconciler';
import type { PriorityLevel } from 'ReactPriorityLevel';
import type { PriorityLevel } from 'ReactPriorityLevel';
import ReactCurrentOwner from 'ReactCurrentOwner';

var {
Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 20 additions & 15 deletions src/renderers/shared/fiber/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
'use strict';

import type { Fiber } from 'ReactFiber';
import type { PriorityLevel } from 'ReactPriorityLevel';
import type { UpdateQueue } from 'ReactFiberUpdateQueue';

var { LowPriority } = require('ReactPriorityLevel');
var {
createUpdateQueue,
addToQueue,
Expand All @@ -27,16 +25,16 @@ 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, priorityLevel : PriorityLevel) {
function scheduleUpdateQueue(fiber: Fiber, updateQueue: UpdateQueue) {
fiber.updateQueue = updateQueue;
// Schedule update on the alternate as well, since we don't know which tree
// is current.
if (fiber.alternate) {
fiber.alternate.updateQueue = updateQueue;
}
scheduleUpdate(fiber, priorityLevel);
scheduleUpdate(fiber);
}

// Class component state updater
Expand All @@ -47,30 +45,27 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
const updateQueue = fiber.updateQueue ?
addToQueue(fiber.updateQueue, partialState) :
createUpdateQueue(partialState);
scheduleUpdateQueue(fiber, updateQueue, LowPriority);
scheduleUpdateQueue(fiber, updateQueue);
Copy link
Collaborator Author

@acdlite acdlite Oct 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found the problem... If scheduleUpdateQueue is sync (and not batched) then the update is flushed before enqueueCallback is ever called. Then enqueueCallback will add a callback to the queue but it won't ever get called until the next time the fiber is committed.

},
enqueueReplaceState(instance, state) {
const fiber = ReactInstanceMap.get(instance);
const updateQueue = createUpdateQueue(state);
updateQueue.isReplace = true;
scheduleUpdateQueue(fiber, updateQueue, LowPriority);
scheduleUpdateQueue(fiber, updateQueue);
},
enqueueForceUpdate(instance) {
const fiber = ReactInstanceMap.get(instance);
const updateQueue = fiber.updateQueue || createUpdateQueue(null);
updateQueue.isForced = true;
scheduleUpdateQueue(fiber, updateQueue, LowPriority);
scheduleUpdateQueue(fiber, updateQueue);
},
enqueueCallback(instance, callback) {
const fiber = ReactInstanceMap.get(instance);
let updateQueue = fiber.updateQueue ?
fiber.updateQueue :
createUpdateQueue(null);
addCallbackToQueue(updateQueue, callback);
fiber.updateQueue = updateQueue;
if (fiber.alternate) {
fiber.alternate.updateQueue = updateQueue;
}
scheduleUpdateQueue(fiber, updateQueue);
},
};

Expand Down Expand Up @@ -131,7 +126,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
// process them now.
const updateQueue = workInProgress.updateQueue;
if (updateQueue) {
instance.state = mergeUpdateQueue(updateQueue, state, props);
instance.state = mergeUpdateQueue(updateQueue, instance, state, props);
}
}
}
Expand Down Expand Up @@ -175,7 +170,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
// process them now.
const newUpdateQueue = workInProgress.updateQueue;
if (newUpdateQueue) {
newInstance.state = mergeUpdateQueue(newUpdateQueue, newState, newProps);
newInstance.state = mergeUpdateQueue(newUpdateQueue, newInstance, newState, newProps);
}
}
return true;
Expand Down Expand Up @@ -212,11 +207,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber, priorityLevel : Priori
// TODO: Previous state can be null.
let newState;
if (updateQueue) {
newState = mergeUpdateQueue(updateQueue, 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,
Expand Down
55 changes: 27 additions & 28 deletions src/renderers/shared/fiber/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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: {
Expand Down
19 changes: 15 additions & 4 deletions src/renderers/shared/fiber/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Loading