From 57cc182f263d2916ed6ac870dd0919da03317a8d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 16 Nov 2016 15:42:39 +0000 Subject: [PATCH] Add Fibers to ReactDOMComponentTree This adds precaching to ReactDOMFiber. I.e. adding a handle from the DOM node to the internal Fiber. This means that we need to expose an internal handle to the reconciler. We use duck typing to figure out if it is a Fiber or Stack instance. The new failing tests are failing because this is now able to actually fire events onto Fibers and then the result of those events are incorrect where as they were ignored before. --- scripts/fiber/tests-failing.txt | 15 +++------ scripts/fiber/tests-passing.txt | 13 +++++--- src/renderers/dom/fiber/ReactDOMFiber.js | 27 +++++++++++++--- .../dom/shared/ReactDOMComponentTree.js | 30 +++++++++++++++-- .../__tests__/ReactDOMComponentTree-test.js | 32 +++++++++++++------ .../shared/fiber/ReactFiberCompleteWork.js | 4 +-- .../shared/fiber/ReactFiberReconciler.js | 8 ++--- 7 files changed, 93 insertions(+), 36 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index f43c39fcb8..787f200407 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -85,10 +85,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should update arbitrary attributes for tags containing dashes * should update styles when `style` changes from null to object * should empty element when removing innerHTML -* should transition from string content to innerHTML -* should transition from innerHTML to string content * should transition from innerHTML to children in nested el -* should transition from children to innerHTML in nested el * should not incur unnecessary DOM mutations for attributes * should not incur unnecessary DOM mutations for string properties * should not incur unnecessary DOM mutations for boolean properties @@ -119,9 +116,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * gives source code refs for unknown prop warning for exact elements * gives source code refs for unknown prop warning for exact elements in composition -src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js -* finds instances for nodes - src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * can reconcile text merged by Node.normalize() alongside other elements * can reconcile text merged by Node.normalize() @@ -134,10 +128,8 @@ src/renderers/dom/shared/__tests__/ReactEventIndependence-test.js * does not when event fired on unmounted tree src/renderers/dom/shared/__tests__/ReactEventListener-test.js -* should propagate events one level down -* should propagate events two levels down -* should not get confused by disappearing elements * should batch between handlers from different roots +* should not fire duplicate events for a React DOM tree src/renderers/dom/shared/__tests__/inputValueTracking-test.js * should return tracker from node @@ -207,6 +199,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should have the correct target value * should control radio buttons * should control radio buttons if the tree updates during render +* should have a this value of undefined if bind is not used * sets type, step, min, max before value always * sets value properly with type coming later in props * does not raise a validation warning when it switches types @@ -214,7 +207,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should ignore and warn invalid children types -* should be able to use dangerouslySetInnerHTML on option * should set attribute for empty value * should allow ignoring `value` on option @@ -233,6 +225,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should remember updated value when switching to uncontrolled * should not control defaultValue if readding options * should refresh state on change +* should be able to safely remove select onChange * should select grandchild options nested inside an optgroup src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js @@ -404,6 +397,7 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js * should enter from the window to the shallowest * should leave to the window * should leave to the window from the shallowest +* should determine the first common ancestor correctly src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js * should prevent non-function listeners, at dispatch @@ -463,4 +457,5 @@ src/test/__tests__/ReactTestUtils-test.js * should support injected wrapper components as DOM components * should change the value of an input field * should change the value of an input field in a component +* should not warn when simulating events with extra properties * should set the type of the event diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index e7e623d5d9..91ea3b01cb 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -525,6 +525,9 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should skip reserved props on web components * should skip dangerouslySetInnerHTML on web components * should clear all the styles when removing `style` +* should transition from string content to innerHTML +* should transition from innerHTML to string content +* should transition from children to innerHTML in nested el * handles multiple child updates without interference * should generate the correct markup with className * should escape style names and values @@ -537,6 +540,7 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js * finds nodes for instances +* finds instances for nodes src/renderers/dom/shared/__tests__/ReactDOMIDOperations-test.js * should update innerHTML and preserve whitespace @@ -554,7 +558,9 @@ src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js src/renderers/dom/shared/__tests__/ReactEventListener-test.js * should dispatch events from outside React tree -* should not fire duplicate events for a React DOM tree +* should propagate events one level down +* should propagate events two levels down +* should not get confused by disappearing elements src/renderers/dom/shared/__tests__/escapeTextContentForBrowser-test.js * should escape boolean to string @@ -679,7 +685,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should not render name attribute if it is not supplied * should not render name attribute if it is not supplied for SSR * should not set a value for submit buttons unnecessarily -* should have a this value of undefined if bind is not used * should update defaultValue to empty string * should not warn if radio value changes but never becomes controlled * should not warn if radio value changes but never becomes uncontrolled @@ -687,6 +692,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should flatten children to a string * should ignore null/undefined/false children without warning +* should be able to use dangerouslySetInnerHTML on option src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should not throw with `defaultValue` and without children @@ -694,7 +700,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should support server-side rendering * should support server-side rendering with defaultValue * should support server-side rendering with multiple -* should be able to safely remove select onChange src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should not render value as an attribute @@ -960,7 +965,6 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js * should not traverse when traversing outside DOM * should not traverse when enter/leaving outside DOM * should not traverse if enter/leave the same node -* should determine the first common ancestor correctly src/renderers/shared/shared/event/__tests__/EventPluginRegistry-test.js * should be able to inject ordering before plugins @@ -1297,7 +1301,6 @@ src/test/__tests__/ReactTestUtils-test.js * can scryRenderedDOMComponentsWithClass with multiple classes * traverses children in the correct order * should throw when attempting to use ReactTestUtils.Simulate with shallow rendering -* should not warn when simulating events with extra properties * can scry with stateless components involved src/test/__tests__/reactComponentExpect-test.js diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 08fd6d7c59..25075bb415 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -15,10 +15,13 @@ import type { HostChildren } from 'ReactFiberReconciler'; var ReactFiberReconciler = require('ReactFiberReconciler'); +var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var warning = require('warning'); +var { precacheFiberNode } = ReactDOMComponentTree; + type DOMContainerElement = Element & { _reactRootContainer: ?Object }; type Container = Element; @@ -51,8 +54,14 @@ var DOMRenderer = ReactFiberReconciler({ recursivelyAppendChildren(container, children); }, - createInstance(type : string, props : Props, children : HostChildren) : Instance { - const domElement = document.createElement(type); + createInstance( + type : string, + props : Props, + children : HostChildren, + internalInstanceHandle : Object + ) : Instance { + const domElement : Instance = document.createElement(type); + precacheFiberNode(internalInstanceHandle, domElement); recursivelyAppendChildren(domElement, children); if (typeof props.className !== 'undefined') { domElement.className = props.className; @@ -61,6 +70,10 @@ var DOMRenderer = ReactFiberReconciler({ domElement.textContent = props.children; } else if (typeof props.children === 'number') { domElement.textContent = props.children.toString(); + } else if (typeof props.dangerouslySetInnerHTML === 'object' && + props.dangerouslySetInnerHTML !== null && + typeof props.dangerouslySetInnerHTML.__html === 'string') { + domElement.innerHTML = props.dangerouslySetInnerHTML.__html; } return domElement; }, @@ -81,11 +94,17 @@ var DOMRenderer = ReactFiberReconciler({ domElement.textContent = newProps.children; } else if (typeof newProps.children === 'number') { domElement.textContent = newProps.children.toString(); + } else if (typeof newProps.dangerouslySetInnerHTML === 'object' && + newProps.dangerouslySetInnerHTML !== null && + typeof newProps.dangerouslySetInnerHTML.__html === 'string') { + domElement.innerHTML = newProps.dangerouslySetInnerHTML.__html; } }, - createTextInstance(text : string) : TextInstance { - return document.createTextNode(text); + createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance { + var textNode : TextInstance = document.createTextNode(text); + precacheFiberNode(internalInstanceHandle, textNode); + return textNode; }, commitTextUpdate(textInstance : TextInstance, oldText : string, newText : string) : void { diff --git a/src/renderers/dom/shared/ReactDOMComponentTree.js b/src/renderers/dom/shared/ReactDOMComponentTree.js index d440446ace..043f1fedbb 100644 --- a/src/renderers/dom/shared/ReactDOMComponentTree.js +++ b/src/renderers/dom/shared/ReactDOMComponentTree.js @@ -13,6 +13,7 @@ var DOMProperty = require('DOMProperty'); var ReactDOMComponentFlags = require('ReactDOMComponentFlags'); +var { HostComponent, HostText } = require('ReactTypeOfWork'); var invariant = require('invariant'); @@ -59,6 +60,10 @@ function precacheNode(inst, node) { node[internalInstanceKey] = hostInst; } +function precacheFiberNode(hostInst, node) { + node[internalInstanceKey] = hostInst; +} + function uncacheNode(inst) { var node = inst._hostNode; if (node) { @@ -133,7 +138,11 @@ function getClosestInstanceFromNode(node) { } var closest; - var inst; + var inst = node[internalInstanceKey]; + if (inst.tag === HostComponent || inst.tag === HostText) { + // In Fiber, this will always be the deepest root. + return inst; + } for (; node && (inst = node[internalInstanceKey]); node = parents.pop()) { closest = inst; if (parents.length) { @@ -149,7 +158,17 @@ function getClosestInstanceFromNode(node) { * instance, or null if the node was not rendered by this React. */ function getInstanceFromNode(node) { - var inst = getClosestInstanceFromNode(node); + var inst = node[internalInstanceKey]; + if (inst) { + if (inst.tag === HostComponent || inst.tag === HostText) { + return inst; + } else if (inst._hostNode === node) { + return inst; + } else { + return null; + } + } + inst = getClosestInstanceFromNode(node); if (inst != null && inst._hostNode === node) { return inst; } else { @@ -162,6 +181,12 @@ function getInstanceFromNode(node) { * DOM node. */ function getNodeFromInstance(inst) { + if (inst.tag === HostComponent || inst.tag === HostText) { + // In Fiber this, is just the state node right now. We assume it will be + // a host component or host text. + return inst.stateNode; + } + // Without this first invariant, passing a non-DOM-component triggers the next // invariant for a missing parent, which is super confusing. invariant( @@ -200,6 +225,7 @@ var ReactDOMComponentTree = { precacheChildNodes: precacheChildNodes, precacheNode: precacheNode, uncacheNode: uncacheNode, + precacheFiberNode: precacheFiberNode, }; module.exports = ReactDOMComponentTree; diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js index 71733578e0..9c35e59390 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponentTree-test.js @@ -24,6 +24,20 @@ describe('ReactDOMComponentTree', () => { return ReactDOM.render(elt, container); } + function getTypeOf(instance) { + if (typeof instance.tag === 'number') { + return instance.type; + } + return instance._currentElement.type; + } + + function getTextOf(instance) { + if (typeof instance.tag === 'number') { + return instance.memoizedProps; + } + return instance._stringText; + } + beforeEach(() => { React = require('React'); ReactDOM = require('ReactDOM'); @@ -92,20 +106,20 @@ describe('ReactDOMComponentTree', () => { ); } - expect(renderAndGetInstance(null)._currentElement.type).toBe('section'); - expect(renderAndGetInstance('div')._currentElement.type).toBe('div'); - expect(renderAndGetInstance('h1')._currentElement.type).toBe('h1'); - expect(renderAndGetInstance('p')._currentElement.type).toBe('p'); - expect(renderAndGetInstance('input')._currentElement.type).toBe('input'); - expect(renderAndGetInstance('main')._currentElement.type).toBe('main'); + expect(getTypeOf(renderAndGetInstance(null))).toBe('section'); + expect(getTypeOf(renderAndGetInstance('div'))).toBe('div'); + expect(getTypeOf(renderAndGetInstance('h1'))).toBe('h1'); + expect(getTypeOf(renderAndGetInstance('p'))).toBe('p'); + expect(getTypeOf(renderAndGetInstance('input'))).toBe('input'); + expect(getTypeOf(renderAndGetInstance('main'))).toBe('main'); // This one's a text component! var root = renderAndQuery(null); var inst = ReactDOMComponentTree.getInstanceFromNode(root.children[0].childNodes[2]); - expect(inst._stringText).toBe('goodbye.'); + expect(getTextOf(inst)).toBe('goodbye.'); - expect(renderAndGetClosest('b')._currentElement.type).toBe('main'); - expect(renderAndGetClosest('img')._currentElement.type).toBe('main'); + expect(getTypeOf(renderAndGetClosest('b'))).toBe('main'); + expect(getTypeOf(renderAndGetClosest('img'))).toBe('main'); }); }); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 82a9bdf211..12c3fc29e3 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -197,7 +197,7 @@ module.exports = function(config : HostConfig) { } const child = workInProgress.child; const children = (child && !child.sibling) ? (child.output : ?Fiber | I) : child; - const instance = createInstance(workInProgress.type, newProps, children); + const instance = createInstance(workInProgress.type, newProps, children, workInProgress); // TODO: This seems like unnecessary duplication. workInProgress.stateNode = instance; workInProgress.output = instance; @@ -223,7 +223,7 @@ module.exports = function(config : HostConfig) { return null; } } - const textInstance = createTextInstance(newText); + const textInstance = createTextInstance(newText, workInProgress); // TODO: This seems like unnecessary duplication. workInProgress.stateNode = textInstance; workInProgress.output = textInstance; diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index e85f25bc46..820e745149 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -36,6 +36,8 @@ type HostChildNode = { tag: TypeOfWork, output: HostChildren, sibling: any export type HostChildren = null | void | I | HostChildNode; +type OpaqueNode = Fiber; + export type HostConfig = { // TODO: We don't currently have a quick way to detect that children didn't @@ -44,11 +46,11 @@ export type HostConfig = { updateContainer(containerInfo : C, children : HostChildren) : void, - createInstance(type : T, props : P, children : HostChildren) : I, + createInstance(type : T, props : P, children : HostChildren, internalInstanceHandle : OpaqueNode) : I, prepareUpdate(instance : I, oldProps : P, newProps : P) : boolean, commitUpdate(instance : I, oldProps : P, newProps : P) : void, - createTextInstance(text : string) : TI, + createTextInstance(text : string, internalInstanceHandle : OpaqueNode) : TI, commitTextUpdate(textInstance : TI, oldText : string, newText : string) : void, appendChild(parentInstance : I, child : I | TI) : void, @@ -61,8 +63,6 @@ export type HostConfig = { useSyncScheduling ?: boolean, }; -type OpaqueNode = Fiber; - export type Reconciler = { mountContainer(element : ReactElement, containerInfo : C) : OpaqueNode, updateContainer(element : ReactElement, container : OpaqueNode) : void,