From 33602d435a351404bc8a29b2dc461fc376e1cbaf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philipp=20Spie=C3=9F?= <hello@philippspiess.com>
Date: Sun, 12 Aug 2018 17:14:05 +0200
Subject: [PATCH] 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
---
 .../src/client/ReactDOMFiberInput.js          | 36 +++++++------------
 packages/react-dom/src/client/SafeValue.js    | 31 ++++++++++++++++
 2 files changed, 44 insertions(+), 23 deletions(-)
 create mode 100644 packages/react-dom/src/client/SafeValue.js

diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js
index 8547aa50c1f2a..b609b02ff4aa0 100644
--- a/packages/react-dom/src/client/ReactDOMFiberInput.js
+++ b/packages/react-dom/src/client/ReactDOMFiberInput.js
@@ -14,12 +14,15 @@ import warning from 'shared/warning';
 
 import * as DOMPropertyOperations from './DOMPropertyOperations';
 import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
+import {getSafeValue, safeValueToString} from './SafeValue';
 import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';
 import * as inputValueTracking from './inputValueTracking';
 
+import type {SafeValue} from './SafeValue';
+
 type InputWithWrapperState = HTMLInputElement & {
   _wrapperState: {
-    initialValue: string,
+    initialValue: SafeValue,
     initialChecked: ?boolean,
     controlled?: boolean,
   },
@@ -174,13 +177,14 @@ export function updateWrapper(element: Element, props: Object) {
     if (props.type === 'number') {
       if (
         (value === 0 && node.value === '') ||
+        // We explicitly want to coerce to number here if possible.
         // eslint-disable-next-line
-        node.value != value
+        node.value != (value: any)
       ) {
-        node.value = '' + value;
+        node.value = safeValueToString(value);
       }
-    } else if (node.value !== '' + value) {
-      node.value = '' + value;
+    } else if (node.value !== safeValueToString(value)) {
+      node.value = safeValueToString(value);
     }
   }
 
@@ -203,7 +207,7 @@ export function postMountWrapper(
   const node = ((element: any): InputWithWrapperState);
 
   if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
-    const initialValue = '' + node._wrapperState.initialValue;
+    const initialValue = safeValueToString(node._wrapperState.initialValue);
     const currentValue = node.value;
 
     // Do not assign value if it is already set. This prevents user text input
@@ -312,23 +316,9 @@ export function setDefaultValue(
     node.ownerDocument.activeElement !== node
   ) {
     if (value == null) {
-      node.defaultValue = '' + node._wrapperState.initialValue;
-    } else if (node.defaultValue !== '' + value) {
-      node.defaultValue = '' + value;
+      node.defaultValue = safeValueToString(node._wrapperState.initialValue);
+    } else if (node.defaultValue !== safeValueToString(value)) {
+      node.defaultValue = safeValueToString(value);
     }
   }
 }
-
-function getSafeValue(value: *): * {
-  switch (typeof value) {
-    case 'boolean':
-    case 'number':
-    case 'object':
-    case 'string':
-    case 'undefined':
-      return value;
-    default:
-      // function, symbol are assigned as empty strings
-      return '';
-  }
-}
diff --git a/packages/react-dom/src/client/SafeValue.js b/packages/react-dom/src/client/SafeValue.js
new file mode 100644
index 0000000000000..5dca73eaa1ed2
--- /dev/null
+++ b/packages/react-dom/src/client/SafeValue.js
@@ -0,0 +1,31 @@
+/**
+ * Copyright (c) 2013-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export opaque type SafeValue = boolean | number | Object | string | null | void;
+
+// Flow does not allow string concatenation of most non-string types. To work
+// around this limitation, we use an opaque type that can only be obtained by
+// passing the value through getSafeValue first.
+export function safeValueToString(value: SafeValue): string {
+  return '' + (value: any);
+}
+
+export function getSafeValue(value: mixed): SafeValue {
+  switch (typeof value) {
+    case 'boolean':
+    case 'number':
+    case 'object':
+    case 'string':
+    case 'undefined':
+      return value;
+    default:
+      // function, symbol are assigned as empty strings
+      return '';
+  }
+}