Move input valueTracker to DOM nodes (#10208)
* Move input valueTracker to DOM nodes This moves the storage of the input value tracker to the DOM node instead of the wrapperState. This makes it easier to support both Stack and Fiber without out a lot of ugly type casting and logic branches. related: #10207 * run prettier * remove instance accepting methods * rm unused trst method * address feedback * fix naming * fix lint
Jason Quense committed
Jul 19, 2017 at 12:10 UTC
acd9818b9068345b1063ca1c44e5725005cecf84
6 files changed
+96
-130
scripts/fiber/tests-passing.txt
+4
-4
@@ -1748,9 +1748,9 @@ src/renderers/dom/shared/__tests__/findDOMNode-test.js
1748
* findDOMNode should not throw an error when called within a component that is not mounted
1749
1750
src/renderers/dom/shared/__tests__/inputValueTracking-test.js
1751
-* should attach tracker to wrapper state
1752
-* should define `value` on the instance node
1753
-* should define `checked` on the instance node
1751
+* should attach tracker to node
1752
+* should define `value` on the node instance
1753
+* should define `checked` on the node instance
1754
* should initialize with the current value
1755
* should initialize with the current `checked`
1756
* should track value changes
@@ -1758,7 +1758,7 @@ src/renderers/dom/shared/__tests__/inputValueTracking-test.js
1758
* should update value manually
1759
* should coerce value to a string
1760
* should update value if it changed and return result
1761
-* should track value and return true when updating untracked instance
1761
+* should return true when updating untracked instance
1762
* should return tracker from node
1763
* should stop tracking
1764
* does not crash for nodes with custom value property
src/renderers/dom/fiber/ReactDOMFiberComponent.js
+4
-4
@@ -509,13 +509,13 @@ var ReactDOMFiberComponent = {
509
case 'input':
510
// TODO: Make sure we check if this is still unmounted or do any clean
511
// up necessary since we never stop tracking anymore.
512
- inputValueTracking.trackNode((domElement: any));
512
+ inputValueTracking.track((domElement: any));
513
ReactDOMFiberInput.postMountWrapper(domElement, rawProps);
514
break;
515
case 'textarea':
516
// TODO: Make sure we check if this is still unmounted or do any clean
517
// up necessary since we never stop tracking anymore.
518
- inputValueTracking.trackNode((domElement: any));
518
+ inputValueTracking.track((domElement: any));
519
ReactDOMFiberTextarea.postMountWrapper(domElement, rawProps);
520
break;
521
case 'option':
@@ -967,13 +967,13 @@ var ReactDOMFiberComponent = {
967
case 'input':
968
// TODO: Make sure we check if this is still unmounted or do any clean
969
// up necessary since we never stop tracking anymore.
970
- inputValueTracking.trackNode((domElement: any));
970
+ inputValueTracking.track((domElement: any));
971
ReactDOMFiberInput.postMountWrapper(domElement, rawProps);
972
break;
973
case 'textarea':
974
// TODO: Make sure we check if this is still unmounted or do any clean
975
// up necessary since we never stop tracking anymore.
976
- inputValueTracking.trackNode((domElement: any));
976
+ inputValueTracking.track((domElement: any));
977
ReactDOMFiberTextarea.postMountWrapper(domElement, rawProps);
978
break;
979
case 'select':
src/renderers/dom/shared/__tests__/inputValueTracking-test.js
+52
-51
@@ -17,139 +17,140 @@ var ReactTestUtils = require('react-dom/test-utils');
17
// TODO: can we express this test with only public API?
18
var inputValueTracking = require('inputValueTracking');
19
20
+var getTracker = inputValueTracking._getTrackerFromNode;
21
+
22
describe('inputValueTracking', () => {
21
- var input, checkbox, mockComponent;
23
+ var input;
24
25
beforeEach(() => {
26
input = document.createElement('input');
27
input.type = 'text';
26
- checkbox = document.createElement('input');
27
- checkbox.type = 'checkbox';
28
- mockComponent = {_hostNode: input, _wrapperState: {}};
28
});
29
31
- it('should attach tracker to wrapper state', () => {
32
- inputValueTracking.track(mockComponent);
30
+ it('should attach tracker to node', () => {
31
+ var node = ReactTestUtils.renderIntoDocument(<input type="text" />);
32
34
- expect(mockComponent._wrapperState.hasOwnProperty('valueTracker')).toBe(
35
- true,
36
- );
33
+ expect(getTracker(node)).toBeDefined();
34
});
35
39
- it('should define `value` on the instance node', () => {
40
- inputValueTracking.track(mockComponent);
36
+ it('should define `value` on the node instance', () => {
37
+ var node = ReactTestUtils.renderIntoDocument(<input type="text" />);
38
42
- expect(input.hasOwnProperty('value')).toBe(true);
39
+ expect(node.hasOwnProperty('value')).toBe(true);
40
});
41
45
- it('should define `checked` on the instance node', () => {
46
- mockComponent._hostNode = checkbox;
47
- inputValueTracking.track(mockComponent);
42
+ it('should define `checked` on the node instance', () => {
43
+ var node = ReactTestUtils.renderIntoDocument(<input type="checkbox" />);
44
49
- expect(checkbox.hasOwnProperty('checked')).toBe(true);
45
+ expect(node.hasOwnProperty('checked')).toBe(true);
46
});
47
48
it('should initialize with the current value', () => {
49
input.value = 'foo';
50
55
- inputValueTracking.track(mockComponent);
51
+ inputValueTracking.track(input);
52
57
- var tracker = mockComponent._wrapperState.valueTracker;
53
+ var tracker = getTracker(input);
54
55
expect(tracker.getValue()).toEqual('foo');
56
});
57
58
it('should initialize with the current `checked`', () => {
63
- mockComponent._hostNode = checkbox;
59
+ const checkbox = document.createElement('input');
60
+
61
+ checkbox.type = 'checkbox';
62
checkbox.checked = true;
65
- inputValueTracking.track(mockComponent);
63
+ inputValueTracking.track(checkbox);
64
67
- var tracker = mockComponent._wrapperState.valueTracker;
65
+ var tracker = getTracker(checkbox);
66
67
expect(tracker.getValue()).toEqual('true');
68
});
69
70
it('should track value changes', () => {
73
- input.value = 'foo';
74
-
75
- inputValueTracking.track(mockComponent);
71
+ var node = ReactTestUtils.renderIntoDocument(
72
+ <input type="text" defaultValue="foo" />,
73
+ );
74
77
- var tracker = mockComponent._wrapperState.valueTracker;
75
+ var tracker = getTracker(node);
76
79
- input.value = 'bar';
77
+ node.value = 'bar';
78
expect(tracker.getValue()).toEqual('bar');
79
});
80
81
it('should tracked`checked` changes', () => {
84
- mockComponent._hostNode = checkbox;
85
- checkbox.checked = true;
86
- inputValueTracking.track(mockComponent);
82
+ var node = ReactTestUtils.renderIntoDocument(
83
+ <input type="checkbox" defaultChecked={true} />,
84
+ );
85
88
- var tracker = mockComponent._wrapperState.valueTracker;
86
+ var tracker = getTracker(node);
87
90
- checkbox.checked = false;
88
+ node.checked = false;
89
expect(tracker.getValue()).toEqual('false');
90
});
91
92
it('should update value manually', () => {
95
- input.value = 'foo';
96
- inputValueTracking.track(mockComponent);
93
+ var node = ReactTestUtils.renderIntoDocument(
94
+ <input type="text" defaultValue="foo" />,
95
+ );
96
98
- var tracker = mockComponent._wrapperState.valueTracker;
97
+ var tracker = getTracker(node);
98
99
tracker.setValue('bar');
100
expect(tracker.getValue()).toEqual('bar');
101
});
102
103
it('should coerce value to a string', () => {
105
- input.value = 'foo';
106
- inputValueTracking.track(mockComponent);
104
+ var node = ReactTestUtils.renderIntoDocument(
105
+ <input type="text" defaultValue="foo" />,
106
+ );
107
108
- var tracker = mockComponent._wrapperState.valueTracker;
108
+ var tracker = getTracker(node);
109
110
tracker.setValue(500);
111
expect(tracker.getValue()).toEqual('500');
112
});
113
114
it('should update value if it changed and return result', () => {
115
- inputValueTracking.track(mockComponent);
116
- input.value = 'foo';
115
+ var node = ReactTestUtils.renderIntoDocument(
116
+ <input type="text" defaultValue="foo" />,
117
+ );
118
118
- var tracker = mockComponent._wrapperState.valueTracker;
119
+ var tracker = getTracker(node);
120
120
- expect(inputValueTracking.updateValueIfChanged(mockComponent)).toBe(false);
121
+ expect(inputValueTracking.updateValueIfChanged(node)).toBe(false);
122
123
tracker.setValue('bar');
124
124
- expect(inputValueTracking.updateValueIfChanged(mockComponent)).toBe(true);
125
+ expect(inputValueTracking.updateValueIfChanged(node)).toBe(true);
126
127
expect(tracker.getValue()).toEqual('foo');
128
});
129
129
- it('should track value and return true when updating untracked instance', () => {
130
+ it('should return true when updating untracked instance', () => {
131
input.value = 'foo';
132
132
- expect(inputValueTracking.updateValueIfChanged(mockComponent)).toBe(true);
133
+ expect(inputValueTracking.updateValueIfChanged(input)).toBe(true);
134
134
- var tracker = mockComponent._wrapperState.valueTracker;
135
- expect(tracker.getValue()).toEqual('foo');
135
+ expect(getTracker(input)).not.toBeDefined();
136
});
137
138
it('should return tracker from node', () => {
139
var div = document.createElement('div');
140
var node = ReactDOM.render(<input type="text" defaultValue="foo" />, div);
141
- var tracker = inputValueTracking._getTrackerFromNode(node);
141
+
142
+ var tracker = getTracker(node);
143
expect(tracker.getValue()).toEqual('foo');
144
});
145
146
it('should stop tracking', () => {
146
- inputValueTracking.track(mockComponent);
147
+ inputValueTracking.track(input);
148
148
- expect(mockComponent._wrapperState.valueTracker).not.toEqual(null);
149
+ expect(getTracker(input)).not.toEqual(null);
150
150
- inputValueTracking.stopTracking(mockComponent);
151
+ inputValueTracking.stopTracking(input);
152
152
- expect(mockComponent._wrapperState.valueTracker).toEqual(null);
153
+ expect(getTracker(input)).toEqual(null);
154
155
expect(input.hasOwnProperty('value')).toBe(false);
156
});
src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js
+2
-1
@@ -99,7 +99,8 @@ function runEventInBatch(event) {
99
}
100
101
function getInstIfValueChanged(targetInst) {
102
- if (inputValueTracking.updateValueIfChanged(targetInst)) {
102
+ const targetNode = ReactDOMComponentTree.getNodeFromInstance(targetInst);
103
+ if (inputValueTracking.updateValueIfChanged(targetNode)) {
104
return targetInst;
105
}
106
}
src/renderers/dom/shared/inputValueTracking.js
+31
-67
@@ -12,25 +12,15 @@
12
13
'use strict';
14
15
-var {ELEMENT_NODE} = require('HTMLNodeType');
16
-import type {Fiber} from 'ReactFiber';
17
-import type {ReactInstance} from 'ReactInstanceType';
18
-
15
type ValueTracker = {
16
getValue(): string,
17
setValue(value: string): void,
18
stopTracking(): void,
19
};
24
-type WrapperState = {_wrapperState: {valueTracker: ?ValueTracker}};
25
-type ElementWithWrapperState = Element & WrapperState;
26
-type InstanceWithWrapperState = ReactInstance & WrapperState;
27
-type SubjectWithWrapperState =
28
- | InstanceWithWrapperState
29
- | ElementWithWrapperState;
30
-
31
-var ReactDOMComponentTree = require('ReactDOMComponentTree');
20
+type WrapperState = {_valueTracker: ?ValueTracker};
21
+type ElementWithValueTracker = HTMLInputElement & WrapperState;
22
33
-function isCheckable(elem: any) {
23
+function isCheckable(elem: HTMLInputElement) {
24
var type = elem.type;
25
var nodeName = elem.nodeName;
26
return (
@@ -40,26 +30,30 @@ function isCheckable(elem: any) {
30
);
31
}
32
43
-function getTracker(inst: any) {
44
- if (typeof inst.tag === 'number') {
45
- inst = inst.stateNode;
46
- }
47
- return inst._wrapperState.valueTracker;
33
+function getTracker(node: ElementWithValueTracker) {
34
+ return node._valueTracker;
35
}
36
50
-function detachTracker(subject: SubjectWithWrapperState) {
51
- subject._wrapperState.valueTracker = null;
37
+function detachTracker(node: ElementWithValueTracker) {
38
+ node._valueTracker = null;
39
}
40
54
-function getValueFromNode(node: any) {
55
- var value;
56
- if (node) {
57
- value = isCheckable(node) ? '' + node.checked : node.value;
41
+function getValueFromNode(node: HTMLInputElement): string {
42
+ var value = '';
43
+ if (!node) {
44
+ return value;
45
}
46
+
47
+ if (isCheckable(node)) {
48
+ value = node.checked ? 'true' : 'false';
49
+ } else {
50
+ value = node.value;
51
+ }
52
+
53
return value;
54
}
55
62
-function trackValueOnNode(node: any, inst: any): ?ValueTracker {
56
+function trackValueOnNode(node: any): ?ValueTracker {
57
var valueField = isCheckable(node) ? 'checked' : 'value';
58
var descriptor = Object.getOwnPropertyDescriptor(
59
node.constructor.prototype,
@@ -100,7 +94,7 @@ function trackValueOnNode(node: any, inst: any): ?ValueTracker {
94
currentValue = '' + value;
95
},
96
stopTracking() {
103
- detachTracker(inst);
97
+ detachTracker(node);
98
delete node[valueField];
99
},
100
};
@@ -109,70 +103,40 @@ function trackValueOnNode(node: any, inst: any): ?ValueTracker {
103
104
var inputValueTracking = {
105
// exposed for testing
112
- _getTrackerFromNode(node: ElementWithWrapperState) {
113
- return getTracker(ReactDOMComponentTree.getInstanceFromNode(node));
114
- },
106
+ _getTrackerFromNode: getTracker,
107
116
- trackNode(node: ElementWithWrapperState) {
108
+ track(node: ElementWithValueTracker) {
109
if (getTracker(node)) {
110
return;
111
}
120
- node._wrapperState.valueTracker = trackValueOnNode(node, node);
121
- },
112
123
- track(inst: InstanceWithWrapperState) {
124
- if (getTracker(inst)) {
125
- return;
126
- }
127
- var node = ReactDOMComponentTree.getNodeFromInstance(inst);
128
- inst._wrapperState.valueTracker = trackValueOnNode(node, inst);
113
+ // TODO: Once it's just Fiber we can move this to node._wrapperState
114
+ node._valueTracker = trackValueOnNode(node);
115
},
116
131
- // TODO: in practice "subject" can currently be Stack instance,
132
- // Fiber, or DOM node. This is hard to understand. We should either
133
- // make it accept only DOM nodes, or only Fiber/Stack instances.
134
- updateValueIfChanged(subject: SubjectWithWrapperState | Fiber) {
135
- if (!subject) {
117
+ updateValueIfChanged(node: ElementWithValueTracker) {
118
+ if (!node) {
119
return false;
120
}
121
139
- var isNode = (subject: any).nodeType === ELEMENT_NODE;
140
- var tracker = getTracker(subject);
122
+ var tracker = getTracker(node);
123
+ // if there is no tracker at this point it's unlikely
124
+ // that trying again will succeed
125
if (!tracker) {
142
- if (isNode) {
143
- // DOM node
144
- inputValueTracking.trackNode((subject: any));
145
- } else if (typeof (subject: any).tag === 'number') {
146
- // Fiber
147
- inputValueTracking.trackNode((subject: any).stateNode);
148
- } else {
149
- // Stack
150
- inputValueTracking.track((subject: any));
151
- }
126
return true;
127
}
128
155
- var node;
156
- if (isNode) {
157
- // DOM node
158
- node = subject;
159
- } else {
160
- // Fiber and Stack
161
- node = ReactDOMComponentTree.getNodeFromInstance(subject);
162
- }
163
-
129
var lastValue = tracker.getValue();
130
var nextValue = getValueFromNode(node);
131
if (nextValue !== lastValue) {
132
tracker.setValue(nextValue);
133
return true;
134
}
170
-
135
return false;
136
},
137
174
- stopTracking(inst: InstanceWithWrapperState | Fiber) {
175
- var tracker = getTracker(inst);
138
+ stopTracking(node: ElementWithValueTracker) {
139
+ var tracker = getTracker(node);
140
if (tracker) {
141
tracker.stopTracking();
142
}
src/renderers/dom/stack/client/ReactDOMComponent.js
+3
-3
@@ -264,7 +264,7 @@ var mediaEvents = {
264
};
265
266
function trackInputValue() {
267
- inputValueTracking.track(this);
267
+ inputValueTracking.track(getNode(this));
268
}
269
270
function trapClickOnNonInteractiveElement() {
@@ -896,7 +896,7 @@ ReactDOMComponent.Mixin = {
896
ReactDOMInput.updateWrapper(this);
897
// We also check that we haven't missed a value update, such as a
898
// Radio group shifting the checked value to another named radio input.
899
- inputValueTracking.updateValueIfChanged(this);
899
+ inputValueTracking.updateValueIfChanged(getNode(this));
900
break;
901
case 'textarea':
902
ReactDOMTextarea.updateWrapper(this);
@@ -1141,7 +1141,7 @@ ReactDOMComponent.Mixin = {
1141
break;
1142
case 'input':
1143
case 'textarea':
1144
- inputValueTracking.stopTracking(this);
1144
+ inputValueTracking.stopTracking(getNode(this));
1145
break;
1146
case 'html':
1147
case 'head':