@samitouri / QOS-React-2 / commits / 33602d435a

Improve soundness of ReactDOMFiberInput typings (#13367)

* Improve soundness of ReactDOMFiberInput typings This is an attempt in improving the soundness for the safe value cast that was added in #11741. We want this to avoid situations like [this one](https://github.com/facebook/react/pull/13362#discussion_r209380079) where we need to remember why we have certain type casts. Additionally we can be sure that we only cast safe values to string. The problem was `getSafeValue()`. It used the (deprecated) `*` type to infer the type which resulted in a passing-through of the implicit `any` of the props `Object`. So `getSafeValue()` was effectively returning `any`. Once I fixed this, I found out that Flow does not allow concatenating all possible types to a string (e.g `"" + false` fails in Flow). To fix this as well, I've opted into making the SafeValue type opaque and added a function that can be used to get the string value. This is sound because we know that SafeValue is already checked. I've verified that the interim function is inlined by the compiler and also looked at a diff of the compiled react-dom bundles to see if I've regressed anything. Seems like we're good. * Fix typo

Philipp Spieß committed Aug 12, 2018 at 17:14 UTC 33602d435a351404bc8a29b2dc461fc376e1cbaf

2 files changed +44 -23

packages/react-dom/src/client/ReactDOMFiberInput.js

+13 -23

@@ -14,12 +14,15 @@ import warning from 'shared/warning';

15 import * as DOMPropertyOperations from './DOMPropertyOperations';

16 import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';

17 +import {getSafeValue, safeValueToString} from './SafeValue';

18 import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';

19 import * as inputValueTracking from './inputValueTracking';

21 +import type {SafeValue} from './SafeValue';

22 +

23 type InputWithWrapperState = HTMLInputElement & {

24 _wrapperState: {

22 - initialValue: string,

25 + initialValue: SafeValue,

26 initialChecked: ?boolean,

27 controlled?: boolean,

28 },

@@ -174,13 +177,14 @@ export function updateWrapper(element: Element, props: Object) {

177 if (props.type === 'number') {

178 if (

179 (value === 0 && node.value === '') ||

180 + // We explicitly want to coerce to number here if possible.

181 // eslint-disable-next-line

178 - node.value != value

182 + node.value != (value: any)

183 ) {

180 - node.value = '' + value;

184 + node.value = safeValueToString(value);

185 }

182 - } else if (node.value !== '' + value) {

183 - node.value = '' + value;

186 + } else if (node.value !== safeValueToString(value)) {

187 + node.value = safeValueToString(value);

188 }

189 }

190

@@ -203,7 +207,7 @@ export function postMountWrapper(

207 const node = ((element: any): InputWithWrapperState);

208

209 if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {

206 - const initialValue = '' + node._wrapperState.initialValue;

210 + const initialValue = safeValueToString(node._wrapperState.initialValue);

211 const currentValue = node.value;

212

213 // Do not assign value if it is already set. This prevents user text input

@@ -312,23 +316,9 @@ export function setDefaultValue(

316 node.ownerDocument.activeElement !== node

317 ) {

318 if (value == null) {

315 - node.defaultValue = '' + node._wrapperState.initialValue;

316 - } else if (node.defaultValue !== '' + value) {

317 - node.defaultValue = '' + value;

319 + node.defaultValue = safeValueToString(node._wrapperState.initialValue);

320 + } else if (node.defaultValue !== safeValueToString(value)) {

321 + node.defaultValue = safeValueToString(value);

322 }

323 }

324 }

321 -

322 -function getSafeValue(value: *): * {

323 - switch (typeof value) {

324 - case 'boolean':

325 - case 'number':

326 - case 'object':

327 - case 'string':

328 - case 'undefined':

329 - return value;

330 - default:

331 - // function, symbol are assigned as empty strings

332 - return '';

333 - }

334 -}

packages/react-dom/src/client/SafeValue.js new

+31

@@ -0,0 +1,31 @@

1 +/**

3 + *

4 + * This source code is licensed under the MIT license found in the

5 + * LICENSE file in the root directory of this source tree.

6 + *

7 + * @flow

8 + */

9 +

11 +

12 +// Flow does not allow string concatenation of most non-string types. To work

13 +// around this limitation, we use an opaque type that can only be obtained by

14 +// passing the value through getSafeValue first.

15 +export function safeValueToString(value: SafeValue): string {

16 + return '' + (value: any);

17 +}

18 +

19 +export function getSafeValue(value: mixed): SafeValue {

20 + switch (typeof value) {

21 + case 'boolean':

22 + case 'number':

23 + case 'object':

24 + case 'string':

25 + case 'undefined':

26 + return value;

27 + default:

28 + // function, symbol are assigned as empty strings

29 + return '';

30 + }

31 +}