@samitouri / QOS-React / commits / a2ca759dfd

Handle controlled components in Fiber

We need to adjust inputValueTracking a bit to handle the fact that Fiber attaches wrapper state on nodes.

Sebastian Markbage committed Nov 18, 2016 at 15:09 UTC a2ca759dfd4ff4d82124b30b1015d206260b6925
6 files changed +101 -81
scripts/fiber/tests-failing.txt
+1 -31
@@ -14,15 +14,6 @@ src/addons/__tests__/renderSubtreeIntoContainer-test.js
14 src/isomorphic/classic/__tests__/ReactContextValidator-test.js
15 * should pass previous context to lifecycles
16
17 -src/isomorphic/classic/class/__tests__/ReactBind-test.js
18 -* Holds reference to instance
19 -* works with mixins
20 -
21 -src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js
22 -* should work with manual binding
23 -* works with mixins that have not opted out of autobinding
24 -* works with mixins that have opted out of autobinding
25 -
17 src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js
18 * includes the owner name when passing null, undefined, boolean, or number
19
@@ -47,11 +38,7 @@ src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js
38 * should bubble simply
39 * should continue bubbling if an error is thrown
40 * should set currentTarget
50 -* should support stopPropagation()
51 -* should stop after first dispatch if stopPropagation
41 * should not stopPropagation if false is returned
53 -* should invoke handlers that were removed while bubbling
54 -* should not invoke newly inserted handlers while bubbling
42 * should have mouse enter simulated by test utils
43 * should infer onTouchTap from a touchStart/End
44 * should infer onTouchTap from when dragging below threshold
@@ -63,8 +50,6 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
50 * should warn when mutating style
51 * should empty element when removing innerHTML
52 * should transition from innerHTML to children in nested el
66 -* should track input values
67 -* should track textarea values
53 * should warn for children on void elements
54 * should report component containing invalid styles
55 * should clean up input value tracking
@@ -90,11 +75,10 @@ src/renderers/dom/shared/__tests__/ReactEventListener-test.js
75 * should not fire duplicate events for a React DOM tree
76
77 src/renderers/dom/shared/__tests__/inputValueTracking-test.js
93 -* should return tracker from node
78 +* should stop tracking
79
80 src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js
81 * should fire change for checkbox input
97 -* should catch setting the value programmatically
82 * should not fire change when setting the value programmatically
83 * should not fire change when setting checked programmatically
84 * should only fire change for checked radio button once
@@ -134,16 +118,10 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js
118 * should trigger load events
119
120 src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
137 -* should properly control a value even if no event listener exists
121 * should control a value in reentrant events
139 -* should control values in reentrant events with different targets
122 * should update `defaultValue` for uncontrolled input
123 * should update `defaultValue` for uncontrolled date/time input
142 -* should properly control a value of number `0`
124 * should have the correct target value
144 -* should control radio buttons
145 -* should control radio buttons if the tree updates during render
146 -* should have a this value of undefined if bind is not used
125 * should update defaultValue to empty string
126 * sets type, step, min, max before value always
127 * 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
143 * should remember value when switching to uncontrolled
144 * should remember updated value when switching to uncontrolled
145 * should not control defaultValue if readding options
168 -* should refresh state on change
169 -* should be able to safely remove select onChange
146 * should select grandchild options nested inside an optgroup
147
148 src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
@@ -177,7 +153,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
153 * should allow setting `value` to `false`
154 * should allow setting `value` to `objToString`
155 * should not incur unnecessary DOM mutations
180 -* should properly control a value of number `0`
156 * should keep value when switching to uncontrolled element if changed
157
158 src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
@@ -324,7 +299,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js
299
300 src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
301 * should not thrash a server rendered layout with client side one
327 -* should react to state changes from callbacks
302 * should warn about `forceUpdate` on unmounted components
303 * should warn about `setState` on unmounted components
304 * should warn about `setState` in render
@@ -365,7 +339,3 @@ src/renderers/shared/stack/reconciler/__tests__/refs-test.js
339
340 src/test/__tests__/ReactTestUtils-test.js
341 * should support injected wrapper components as DOM components
368 -* should change the value of an input field
369 -* should change the value of an input field in a component
370 -* should not warn when simulating events with extra properties
371 -* should set the type of the event
scripts/fiber/tests-passing.txt
+27 -1
@@ -138,11 +138,16 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js
138 * should pass next context to lifecycles
139
140 src/isomorphic/classic/class/__tests__/ReactBind-test.js
141 +* Holds reference to instance
142 +* works with mixins
143 * warns if you try to bind to this
144 * does not warn if you pass an auto-bound method to setState
145
146 src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js
147 +* should work with manual binding
148 * should not hold reference to instance
149 +* works with mixins that have not opted out of autobinding
150 +* works with mixins that have opted out of autobinding
151 * does not warn if you try to bind to this
152 * does not warn if you pass an manually bound method to setState
153
@@ -522,6 +527,10 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
527 * should support custom attributes
528
529 src/renderers/dom/shared/__tests__/ReactBrowserEventEmitter-test.js
530 +* should support stopPropagation()
531 +* should stop after first dispatch if stopPropagation
532 +* should invoke handlers that were removed while bubbling
533 +* should not invoke newly inserted handlers while bubbling
534 * should listen to events only once
535 * should work with event plugins without dependencies
536 * should work with event plugins with dependencies
@@ -571,6 +580,8 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
580 * should warn about contentEditable and children
581 * should respect suppressContentEditableWarning
582 * should validate against invalid styles
583 +* should track input values
584 +* should track textarea values
585 * should support custom elements which extend native elements
586 * should warn against children for void elements
587 * should warn against dangerouslySetInnerHTML for void elements
@@ -626,7 +637,7 @@ src/renderers/dom/shared/__tests__/inputValueTracking-test.js
637 * should coerce value to a string
638 * should update value if it changed and return result
639 * should track value and return true when updating untracked instance
629 -* should stop tracking
640 +* should return tracker from node
641
642 src/renderers/dom/shared/__tests__/quoteAttributeValueForBrowser-test.js
643 * should escape boolean to string
@@ -644,6 +655,7 @@ src/renderers/dom/shared/eventPlugins/__tests__/BeforeInputEventPlugin-test.js
655 * extract onBeforeInput from fallback objects
656
657 src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js
658 +* should catch setting the value programmatically
659 * should unmount
660
661 src/renderers/dom/shared/eventPlugins/__tests__/EnterLeaveEventPlugin-test.js
@@ -727,6 +739,8 @@ src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js
739 * sets innerHTML on it
740
741 src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
742 +* should properly control a value even if no event listener exists
743 +* should control values in reentrant events with different targets
744 * should display `defaultValue` of number 0
745 * only assigns defaultValue if it changes
746 * should display "true" for `defaultValue` of `true`
@@ -744,8 +758,12 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
758 * should allow setting `value` to `false`
759 * should allow setting `value` to `objToString`
760 * should not incur unnecessary DOM mutations
761 +* should properly control a value of number `0`
762 * should not set a value for submit buttons unnecessarily
763 +* should control radio buttons
764 +* should control radio buttons if the tree updates during render
765 * should warn with value and no onChange handler and readOnly specified
766 +* should have a this value of undefined if bind is not used
767 * should warn with checked and no onChange handler with readOnly specified
768 * should warn if checked and defaultChecked props are specified
769 * should warn if value and defaultValue props are specified
@@ -772,7 +790,9 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMSelect-test.js
790 * should support server-side rendering
791 * should support server-side rendering with defaultValue
792 * should support server-side rendering with multiple
793 +* should refresh state on change
794 * should warn if value and defaultValue props are specified
795 +* should be able to safely remove select onChange
796
797 src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
798 * should allow setting `defaultValue`
@@ -785,6 +805,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMTextarea-test.js
805 * should render value for SSR
806 * should take updates to `defaultValue` for uncontrolled textarea
807 * should take updates to children in lieu of `defaultValue` for uncontrolled textarea
808 +* should properly control a value of number `0`
809 * should treat children like `defaultValue`
810 * should keep value when switching to uncontrolled element if not changed
811 * should allow numbers as children
@@ -1130,6 +1151,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js
1151 src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js
1152 * should support module pattern components
1153 * should support rendering to different child types over time
1154 +* should react to state changes from callbacks
1155 * should rewire refs when rendering to different child types
1156 * should not cache old DOM nodes when switching constructors
1157 * should auto bind methods and values correctly
@@ -1406,8 +1428,12 @@ src/test/__tests__/ReactTestUtils-test.js
1428 * can scryRenderedDOMComponentsWithClass with className contains \n
1429 * can scryRenderedDOMComponentsWithClass with multiple classes
1430 * traverses children in the correct order
1431 +* should change the value of an input field
1432 +* should change the value of an input field in a component
1433 * should throw when attempting to use ReactTestUtils.Simulate with shallow rendering
1434 +* should not warn when simulating events with extra properties
1435 * can scry with stateless components involved
1436 +* should set the type of the event
1437
1438 src/test/__tests__/reactComponentExpect-test.js
1439 * should match composite components
src/renderers/dom/fiber/ReactDOMFiber.js
+4
@@ -14,6 +14,7 @@
14
15 import type { HostChildren } from 'ReactFiberReconciler';
16
17 +var ReactControlledComponent = require('ReactControlledComponent');
18 var ReactFiberReconciler = require('ReactFiberReconciler');
19 var ReactDOMComponentTree = require('ReactDOMComponentTree');
20 var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
@@ -21,6 +22,9 @@ var ReactDOMFiberComponent = require('ReactDOMFiberComponent');
22 var ReactDOMInjection = require('ReactDOMInjection');
23
24 ReactDOMInjection.inject();
25 +ReactControlledComponent.injection.injectFiberControlledHostComponent(
26 + ReactDOMFiberComponent
27 +);
28
29 var warning = require('warning');
30
src/renderers/dom/fiber/ReactDOMFiberComponent.js
+3 -3
@@ -33,7 +33,7 @@ var invariant = require('invariant');
33 var isEventSupported = require('isEventSupported');
34 var setInnerHTML = require('setInnerHTML');
35 var setTextContent = require('setTextContent');
36 -// var inputValueTracking = require('inputValueTracking');
36 +var inputValueTracking = require('inputValueTracking');
37 var warning = require('warning');
38 var didWarnShadyDOM = false;
39
@@ -552,7 +552,7 @@ var ReactDOMFiberComponent = {
552 props = ReactDOMFiberInput.getHostProps(domElement, props);
553 // TODO: Make sure we check if this is still unmounted or do any clean
554 // up necessary since we never stop tracking anymore.
555 - //inputValueTracking.track(domElement); // TODO
555 + inputValueTracking.trackNode(domElement);
556 trapBubbledEventsLocal(domElement, tag);
557 // For controlled components we always need to ensure we're listening
558 // to onChange. Even if there is no listener.
@@ -573,7 +573,7 @@ var ReactDOMFiberComponent = {
573 case 'textarea':
574 ReactDOMFiberTextarea.mountWrapper(domElement, props);
575 props = ReactDOMFiberTextarea.getHostProps(domElement, props);
576 - //inputValueTracking.track(domElement); // TODO
576 + inputValueTracking.trackNode(domElement);
577 trapBubbledEventsLocal(domElement, tag);
578 // For controlled components we always need to ensure we're listening
579 // to onChange. Even if there is no listener.
src/renderers/dom/shared/inputValueTracking.js
+59 -45
@@ -22,6 +22,9 @@ function isCheckable(elem) {
22 }
23
24 function getTracker(inst) {
25 + if (typeof inst.tag === 'number') {
26 + inst = inst.stateNode;
27 + }
28 return inst._wrapperState.valueTracker;
29 }
30
@@ -43,6 +46,54 @@ function getValueFromNode(node) {
46 return value;
47 }
48
49 +function trackValueOnNode(node) {
50 + var valueField = isCheckable(node) ? 'checked' : 'value';
51 + var descriptor = Object.getOwnPropertyDescriptor(
52 + node.constructor.prototype,
53 + valueField
54 + );
55 +
56 + var currentValue = '' + node[valueField];
57 +
58 + // if someone has already defined a value or Safari, then bail
59 + // and don't track value will cause over reporting of changes,
60 + // but it's better then a hard failure
61 + // (needed for certain tests that spyOn input values and Safari)
62 + if (
63 + node.hasOwnProperty(valueField) ||
64 + typeof descriptor.get !== 'function' ||
65 + typeof descriptor.set !== 'function'
66 + ) {
67 + return;
68 + }
69 +
70 + Object.defineProperty(node, valueField, {
71 + enumerable: descriptor.enumerable,
72 + configurable: true,
73 + get: function() {
74 + return descriptor.get.call(this);
75 + },
76 + set: function(value) {
77 + currentValue = '' + value;
78 + descriptor.set.call(this, value);
79 + },
80 + });
81 +
82 + var tracker = {
83 + getValue() {
84 + return currentValue;
85 + },
86 + setValue(value) {
87 + currentValue = '' + value;
88 + },
89 + stopTracking() {
90 + detachTracker(inst);
91 + delete node[valueField];
92 + },
93 + };
94 + return tracker;
95 +}
96 +
97 var inputValueTracking = {
98 // exposed for testing
99 _getTrackerFromNode(node) {
@@ -51,56 +102,19 @@ var inputValueTracking = {
102 );
103 },
104
54 - track: function(inst) {
55 - if (getTracker(inst)) {
105 + trackNode: function(node) {
106 + if (node._wrapperState.valueTracker) {
107 return;
108 }
109 + node._wrapperState.valueTracker = trackValueOnNode(node);
110 + },
111
59 - var node = ReactDOMComponentTree.getNodeFromInstance(inst);
60 - var valueField = isCheckable(node) ? 'checked' : 'value';
61 - var descriptor = Object.getOwnPropertyDescriptor(
62 - node.constructor.prototype,
63 - valueField
64 - );
65 -
66 - var currentValue = '' + node[valueField];
67 -
68 - // if someone has already defined a value or Safari, then bail
69 - // and don't track value will cause over reporting of changes,
70 - // but it's better then a hard failure
71 - // (needed for certain tests that spyOn input values and Safari)
72 - if (
73 - node.hasOwnProperty(valueField) ||
74 - typeof descriptor.get !== 'function' ||
75 - typeof descriptor.set !== 'function'
76 - ) {
112 + track: function(inst) {
113 + if (getTracker(inst)) {
114 return;
115 }
79 -
80 - Object.defineProperty(node, valueField, {
81 - enumerable: descriptor.enumerable,
82 - configurable: true,
83 - get: function() {
84 - return descriptor.get.call(this);
85 - },
86 - set: function(value) {
87 - currentValue = '' + value;
88 - descriptor.set.call(this, value);
89 - },
90 - });
91 -
92 - attachTracker(inst, {
93 - getValue() {
94 - return currentValue;
95 - },
96 - setValue(value) {
97 - currentValue = '' + value;
98 - },
99 - stopTracking() {
100 - detachTracker(inst);
101 - delete node[valueField];
102 - },
103 - });
116 + var node = ReactDOMComponentTree.getNodeFromInstance(inst);
117 + attachTracker(inst, trackValueOnNode(node));
118 },
119
120 updateValueIfChanged(inst) {
src/renderers/shared/shared/event/ReactControlledComponent.js
+7 -1
@@ -36,7 +36,13 @@ function restoreStateOfTarget(internalInstance) {
36 'Fiber needs to be injected to handle a fiber target for controlled ' +
37 'events.'
38 );
39 - fiberHostComponent.restoreControlledState(internalInstance);
39 + // TODO: Ensure that this instance is the current one. Props needs to be correct.
40 + fiberHostComponent.restoreControlledState(
41 + internalInstance.stateNode,
42 + internalInstance.type,
43 + internalInstance.memoizedProps
44 + );
45 + return;
46 }
47 invariant(
48 typeof internalInstance.restoreControlledState === 'function',