From a2ca759dfd4ff4d82124b30b1015d206260b6925 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 18 Nov 2016 15:09:18 +0000 Subject: [PATCH] Handle controlled components in Fiber We need to adjust inputValueTracking a bit to handle the fact that Fiber attaches wrapper state on nodes. --- scripts/fiber/tests-failing.txt | 32 +----- scripts/fiber/tests-passing.txt | 28 ++++- src/renderers/dom/fiber/ReactDOMFiber.js | 4 + .../dom/fiber/ReactDOMFiberComponent.js | 6 +- .../dom/shared/inputValueTracking.js | 104 ++++++++++-------- .../shared/event/ReactControlledComponent.js | 8 +- 6 files changed, 101 insertions(+), 81 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 9f175a9402..5508db8337 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -14,15 +14,6 @@ src/addons/__tests__/renderSubtreeIntoContainer-test.js src/isomorphic/classic/__tests__/ReactContextValidator-test.js * should pass previous context to lifecycles -src/isomorphic/classic/class/__tests__/ReactBind-test.js -* Holds reference to instance -* works with mixins - -src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js -* should work with manual binding -* works with mixins that have not opted out of autobinding -* works with mixins that have opted out of autobinding - src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js * includes the owner name when passing null, undefined, boolean, or number @@ -47,11 +38,7 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js * should bubble simply * should continue bubbling if an error is thrown * should set currentTarget -* should support stopPropagation() -* should stop after first dispatch if stopPropagation * should not stopPropagation if false is returned -* should invoke handlers that were removed while bubbling -* should not invoke newly inserted handlers while bubbling * should have mouse enter simulated by test utils * should infer onTouchTap from a touchStart/End * should infer onTouchTap from when dragging below threshold @@ -63,8 +50,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn when mutating style * should empty element when removing innerHTML * should transition from innerHTML to children in nested el -* should track input values -* should track textarea values * should warn for children on void elements * should report component containing invalid styles * should clean up input value tracking @@ -90,11 +75,10 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js * should not fire duplicate events for a React DOM tree src/renderers/dom/shared/__tests__/inputValueTracking-test.js -* should return tracker from node +* should stop tracking src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js * should fire change for checkbox input -* should catch setting the value programmatically * should not fire change when setting the value programmatically * should not fire change when setting checked programmatically * should only fire change for checked radio button once @@ -134,16 +118,10 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js * should trigger load events 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 update `defaultValue` for uncontrolled input * should update `defaultValue` for uncontrolled date/time input -* should properly control a value of number `0` * 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 * should update defaultValue to empty string * sets type, step, min, max before value always * resets value of date/time input to fix bugs in iOS Safari @@ -165,8 +143,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js * should remember value when switching to uncontrolled * 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 @@ -177,7 +153,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should allow setting `value` to `false` * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations -* should properly control a value of number `0` * should keep value when switching to uncontrolled element if changed src/renderers/dom/stack/client/__tests__/ReactDOM-test.js @@ -324,7 +299,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should not thrash a server rendered layout with client side one -* should react to state changes from callbacks * should warn about `forceUpdate` on unmounted components * should warn about `setState` on unmounted components * should warn about `setState` in render @@ -365,7 +339,3 @@ src/renderers/shared/stack/reconciler/__tests__/refs-test.js 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 92c5cedbe8..369ffa6dcc 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -138,11 +138,16 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js * should pass next context to lifecycles src/isomorphic/classic/class/__tests__/ReactBind-test.js +* Holds reference to instance +* works with mixins * warns if you try to bind to this * does not warn if you pass an auto-bound method to setState src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js +* should work with manual binding * should not hold reference to instance +* works with mixins that have not opted out of autobinding +* works with mixins that have opted out of autobinding * does not warn if you try to bind to this * does not warn if you pass an manually bound method to setState @@ -522,6 +527,10 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should support custom attributes src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js +* should support stopPropagation() +* should stop after first dispatch if stopPropagation +* should invoke handlers that were removed while bubbling +* should not invoke newly inserted handlers while bubbling * should listen to events only once * should work with event plugins without dependencies * should work with event plugins with dependencies @@ -571,6 +580,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn about contentEditable and children * should respect suppressContentEditableWarning * should validate against invalid styles +* should track input values +* should track textarea values * should support custom elements which extend native elements * should warn against children for void elements * should warn against dangerouslySetInnerHTML for void elements @@ -626,7 +637,7 @@ src/renderers/dom/shared/__tests__/inputValueTracking-test.js * should coerce value to a string * should update value if it changed and return result * should track value and return true when updating untracked instance -* should stop tracking +* should return tracker from node src/renderers/dom/shared/__tests__/quoteAttributeValueForBrowser-test.js * should escape boolean to string @@ -644,6 +655,7 @@ src/renderers/dom/shared/eventPlugins/__tests__/BeforeInputEventPlugin-test.js * extract onBeforeInput from fallback objects src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js +* should catch setting the value programmatically * should unmount src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js @@ -727,6 +739,8 @@ src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js * sets innerHTML on it src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +* should properly control a value even if no event listener exists +* should control values in reentrant events with different targets * should display `defaultValue` of number 0 * only assigns defaultValue if it changes * should display "true" for `defaultValue` of `true` @@ -744,8 +758,12 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should allow setting `value` to `false` * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations +* should properly control a value of number `0` * should not set a value for submit buttons unnecessarily +* should control radio buttons +* should control radio buttons if the tree updates during render * should warn with value and no onChange handler and readOnly specified +* should have a this value of undefined if bind is not used * should warn with checked and no onChange handler with readOnly specified * should warn if checked and defaultChecked props are specified * should warn if value and defaultValue props are specified @@ -772,7 +790,9 @@ 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 refresh state on change * should warn if value and defaultValue props are specified +* should be able to safely remove select onChange src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should allow setting `defaultValue` @@ -785,6 +805,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js * should render value for SSR * should take updates to `defaultValue` for uncontrolled textarea * should take updates to children in lieu of `defaultValue` for uncontrolled textarea +* should properly control a value of number `0` * should treat children like `defaultValue` * should keep value when switching to uncontrolled element if not changed * should allow numbers as children @@ -1130,6 +1151,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should support module pattern components * should support rendering to different child types over time +* should react to state changes from callbacks * should rewire refs when rendering to different child types * should not cache old DOM nodes when switching constructors * should auto bind methods and values correctly @@ -1406,8 +1428,12 @@ src/test/__tests__/ReactTestUtils-test.js * can scryRenderedDOMComponentsWithClass with className contains \n * can scryRenderedDOMComponentsWithClass with multiple classes * traverses children in the correct order +* should change the value of an input field +* should change the value of an input field in a component * 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 +* should set the type of the event src/test/__tests__/reactComponentExpect-test.js * should match composite components diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index b7e7a54db6..1e1516aeec 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -14,6 +14,7 @@ import type { HostChildren } from 'ReactFiberReconciler'; +var ReactControlledComponent = require('ReactControlledComponent'); var ReactFiberReconciler = require('ReactFiberReconciler'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); @@ -21,6 +22,9 @@ var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMInjection = require('ReactDOMInjection'); ReactDOMInjection.inject(); +ReactControlledComponent.injection.injectFiberControlledHostComponent( + ReactDOMFiberComponent +); var warning = require('warning'); diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 186d2f0b5f..a39c5c3bdb 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -33,7 +33,7 @@ var invariant = require('invariant'); var isEventSupported = require('isEventSupported'); var setInnerHTML = require('setInnerHTML'); var setTextContent = require('setTextContent'); -// var inputValueTracking = require('inputValueTracking'); +var inputValueTracking = require('inputValueTracking'); var warning = require('warning'); var didWarnShadyDOM = false; @@ -552,7 +552,7 @@ var ReactDOMFiberComponent = { props = ReactDOMFiberInput.getHostProps(domElement, props); // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. - //inputValueTracking.track(domElement); // TODO + inputValueTracking.trackNode(domElement); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. @@ -573,7 +573,7 @@ var ReactDOMFiberComponent = { case 'textarea': ReactDOMFiberTextarea.mountWrapper(domElement, props); props = ReactDOMFiberTextarea.getHostProps(domElement, props); - //inputValueTracking.track(domElement); // TODO + inputValueTracking.trackNode(domElement); trapBubbledEventsLocal(domElement, tag); // For controlled components we always need to ensure we're listening // to onChange. Even if there is no listener. diff --git a/src/renderers/dom/shared/inputValueTracking.js b/src/renderers/dom/shared/inputValueTracking.js index d69f31b383..257b81d47a 100644 --- a/src/renderers/dom/shared/inputValueTracking.js +++ b/src/renderers/dom/shared/inputValueTracking.js @@ -22,6 +22,9 @@ function isCheckable(elem) { } function getTracker(inst) { + if (typeof inst.tag === 'number') { + inst = inst.stateNode; + } return inst._wrapperState.valueTracker; } @@ -43,6 +46,54 @@ function getValueFromNode(node) { return value; } +function trackValueOnNode(node) { + var valueField = isCheckable(node) ? 'checked' : 'value'; + var descriptor = Object.getOwnPropertyDescriptor( + node.constructor.prototype, + valueField + ); + + var currentValue = '' + node[valueField]; + + // if someone has already defined a value or Safari, then bail + // and don't track value will cause over reporting of changes, + // but it's better then a hard failure + // (needed for certain tests that spyOn input values and Safari) + if ( + node.hasOwnProperty(valueField) || + typeof descriptor.get !== 'function' || + typeof descriptor.set !== 'function' + ) { + return; + } + + Object.defineProperty(node, valueField, { + enumerable: descriptor.enumerable, + configurable: true, + get: function() { + return descriptor.get.call(this); + }, + set: function(value) { + currentValue = '' + value; + descriptor.set.call(this, value); + }, + }); + + var tracker = { + getValue() { + return currentValue; + }, + setValue(value) { + currentValue = '' + value; + }, + stopTracking() { + detachTracker(inst); + delete node[valueField]; + }, + }; + return tracker; +} + var inputValueTracking = { // exposed for testing _getTrackerFromNode(node) { @@ -51,56 +102,19 @@ var inputValueTracking = { ); }, + trackNode: function(node) { + if (node._wrapperState.valueTracker) { + return; + } + node._wrapperState.valueTracker = trackValueOnNode(node); + }, + track: function(inst) { if (getTracker(inst)) { return; } - var node = ReactDOMComponentTree.getNodeFromInstance(inst); - var valueField = isCheckable(node) ? 'checked' : 'value'; - var descriptor = Object.getOwnPropertyDescriptor( - node.constructor.prototype, - valueField - ); - - var currentValue = '' + node[valueField]; - - // if someone has already defined a value or Safari, then bail - // and don't track value will cause over reporting of changes, - // but it's better then a hard failure - // (needed for certain tests that spyOn input values and Safari) - if ( - node.hasOwnProperty(valueField) || - typeof descriptor.get !== 'function' || - typeof descriptor.set !== 'function' - ) { - return; - } - - Object.defineProperty(node, valueField, { - enumerable: descriptor.enumerable, - configurable: true, - get: function() { - return descriptor.get.call(this); - }, - set: function(value) { - currentValue = '' + value; - descriptor.set.call(this, value); - }, - }); - - attachTracker(inst, { - getValue() { - return currentValue; - }, - setValue(value) { - currentValue = '' + value; - }, - stopTracking() { - detachTracker(inst); - delete node[valueField]; - }, - }); + attachTracker(inst, trackValueOnNode(node)); }, updateValueIfChanged(inst) { diff --git a/src/renderers/shared/shared/event/ReactControlledComponent.js b/src/renderers/shared/shared/event/ReactControlledComponent.js index c218a22059..29457eb079 100644 --- a/src/renderers/shared/shared/event/ReactControlledComponent.js +++ b/src/renderers/shared/shared/event/ReactControlledComponent.js @@ -36,7 +36,13 @@ function restoreStateOfTarget(internalInstance) { 'Fiber needs to be injected to handle a fiber target for controlled ' + 'events.' ); - fiberHostComponent.restoreControlledState(internalInstance); + // TODO: Ensure that this instance is the current one. Props needs to be correct. + fiberHostComponent.restoreControlledState( + internalInstance.stateNode, + internalInstance.type, + internalInstance.memoizedProps + ); + return; } invariant( typeof internalInstance.restoreControlledState === 'function',