Skip to content

Commit

Permalink
[Fiber] Compute the Host Diff During Reconciliation (#8607)
Browse files Browse the repository at this point in the history
* Allow renderers to return an update payload in prepareUpdate

This then gets stored on updateQueue so that the renderer doesn't need to
think about how to store this.

It then gets passed into commitUpdate during the commit phase.

This allows renderers to do the diffing during the time sliced path,
allocate a queue for changes and do only the absolute minimal work to
apply those changes in the commit phase.

If refs update we still schedule and update.

* Hack around the listener problem

* Diff ReactDOMFiber properties in prepareUpdate

We now take advantage of the new capability to diff properties early.
We do this by generating an update payload in the form of an array with
each property name and value that we're about to update.

* Add todo for handling wasCustomComponentTag

* Always force an update to wrapper components

Wrapper components have custom logic that gets applied at the commit phase
so we always need to ensure that we schedule an update for them.

* Remove rootContainerInstance from commitMount

No use case yet and I removed it from commitUpdate earlier.

* Use update signal object in test renderer

* Incorporate 8652 into new algorithm

* Fix comment

* Add failing test for flipping event handlers

This illustrates the problem that happens if we store a pointer to the
Fiber and then choose not to update that pointer when no properties change.
That causes an old Fiber to be retained on the DOM node. Then that Fiber
can be reused by the pooling mechanism which then will mutate that Fiber
with new event handlers, which makes them active before commit.

* Store current props in the RN instance cache and on the DOM node

This represents the current set of event listeners. By not relying on the
Fiber, it allows us to avoid doing any effects in the commit phase when
nothing changes.

This is a bit ugly. Not super happy how this all came together.
  • Loading branch information
sebmarkbage authored Jan 20, 2017
1 parent c6a7dc7 commit b354db2
Show file tree
Hide file tree
Showing 20 changed files with 412 additions and 173 deletions.
3 changes: 3 additions & 0 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ src/renderers/dom/shared/__tests__/ReactRenderDocument-test.js
* should throw on full document render w/ no markup
* supports findDOMNode on full-page components

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should control a value in reentrant events

src/renderers/shared/__tests__/ReactPerf-test.js
* should count no-op update as waste
* should count no-op update in child as waste
Expand Down
2 changes: 1 addition & 1 deletion scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* findDOMNode should find dom element after expanding a fragment
* should bubble events from the portal to the parent
* should not onMouseLeave when staying in the portal
* should not update event handlers until commit
* should not crash encountering low-priority tree
* throws if non-element passed to top-level render
* throws if something other than false, null, or an element is returned from render
Expand Down Expand Up @@ -959,7 +960,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should properly control a value even if no event listener exists
* should control a value in reentrant events
* should control values in reentrant events with different targets
* should display `defaultValue` of number 0
* only assigns defaultValue if it changes
Expand Down
6 changes: 4 additions & 2 deletions src/renderers/art/ReactARTFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const TYPES = {
TEXT: 'Text',
};

const UPDATE_SIGNAL = {};

/** Helper Methods */

function addEventListeners(instance, type, listener) {
Expand Down Expand Up @@ -418,7 +420,7 @@ const ARTRenderer = ReactFiberReconciler({
// Noop
},

commitUpdate(instance, type, oldProps, newProps) {
commitUpdate(instance, updatePayload, type, oldProps, newProps) {
instance._applyProps(instance, newProps, oldProps);
},

Expand Down Expand Up @@ -482,7 +484,7 @@ const ARTRenderer = ReactFiberReconciler({
},

prepareUpdate(domElement, type, oldProps, newProps) {
return true;
return UPDATE_SIGNAL;
},

removeChild(parentInstance, child) {
Expand Down
24 changes: 15 additions & 9 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@ var {
createElement,
getChildNamespace,
setInitialProperties,
diffProperties,
updateProperties,
} = ReactDOMFiberComponent;
var { precacheFiberNode } = ReactDOMComponentTree;
var {
precacheFiberNode,
updateFiberEventHandlers,
} = ReactDOMComponentTree;

if (__DEV__) {
var validateDOMNesting = require('validateDOMNesting');
Expand Down Expand Up @@ -184,6 +188,7 @@ var DOMRenderer = ReactFiberReconciler({
}
const domElement : Instance = createElement(type, props, rootContainerInstance, parentNamespace);
precacheFiberNode(internalInstanceHandle, domElement);
updateFiberEventHandlers(domElement, props);
return domElement;
},

Expand All @@ -206,8 +211,9 @@ var DOMRenderer = ReactFiberReconciler({
type : string,
oldProps : Props,
newProps : Props,
rootContainerInstance : Container,
hostContext : HostContext,
) : boolean {
) : null | Array<mixed> {
if (__DEV__) {
const hostContextDev = ((hostContext : any) : HostContextDev);
if (typeof newProps.children !== typeof oldProps.children && (
Expand All @@ -218,31 +224,31 @@ var DOMRenderer = ReactFiberReconciler({
validateDOMNesting(null, String(newProps.children), null, ownAncestorInfo);
}
}
return true;
return diffProperties(domElement, type, oldProps, newProps, rootContainerInstance);
},

commitMount(
domElement : Instance,
type : string,
newProps : Props,
rootContainerInstance : Container,
internalInstanceHandle : Object,
) : void {
((domElement : any) : HTMLButtonElement | HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement).focus();
},

commitUpdate(
domElement : Instance,
updatePayload : Array<mixed>,
type : string,
oldProps : Props,
newProps : Props,
rootContainerInstance : Container,
internalInstanceHandle : Object,
) : void {
// Update the internal instance handle so that we know which props are
// the current ones.
precacheFiberNode(internalInstanceHandle, domElement);
updateProperties(domElement, type, oldProps, newProps, rootContainerInstance);
// Update the props handle so that we know which props are the ones with
// with current event handlers.
updateFiberEventHandlers(domElement, newProps);
// Apply the diff to the DOM node.
updateProperties(domElement, updatePayload, type, oldProps, newProps);
},

shouldSetTextContent(props : Props) : boolean {
Expand Down
Loading

0 comments on commit b354db2

Please sign in to comment.