Skip to content

Commit

Permalink
Rename SafeValue to ToStringValue (facebook#13376)
Browse files Browse the repository at this point in the history
Following up on the changes I made in facebook#13367, @gaearon suggest that
"safe" could be read as necessary for security. To avoid misleading a
reader, I'm changing the name.

A few names where discussed in the previous PR. I think `ToStringValue`
makes sense since the value itself is not a string yet but an opaque
type that can be cast to a string. For the actual string concatenation,
I've used `toString` now to avoid confusion: `toStringValueToString`
is super weird and it's namespaced anyhow.

Definitely open for suggestions here. :) I'll wait until we wrap up
facebook#13362 and take care of rebase afterwards.
  • Loading branch information
philipp-spiess authored and Tejas Kumar committed Aug 26, 2018
1 parent ba89027 commit da6b9e0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 17 deletions.
26 changes: 13 additions & 13 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Expand Up @@ -14,15 +14,15 @@ import warning from 'shared/warning';

import * as DOMPropertyOperations from './DOMPropertyOperations';
import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
import {getSafeValue, safeValueToString} from './SafeValue';
import {getToStringValue, toString} from './ToStringValue';
import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';
import * as inputValueTracking from './inputValueTracking';

import type {SafeValue} from './SafeValue';
import type {ToStringValue} from './ToStringValue';

type InputWithWrapperState = HTMLInputElement & {
_wrapperState: {
initialValue: SafeValue,
initialValue: ToStringValue,
initialChecked: ?boolean,
controlled?: boolean,
},
Expand Down Expand Up @@ -117,7 +117,7 @@ export function initWrapperState(element: Element, props: Object) {
node._wrapperState = {
initialChecked:
props.checked != null ? props.checked : props.defaultChecked,
initialValue: getSafeValue(
initialValue: getToStringValue(
props.value != null ? props.value : defaultValue,
),
controlled: isControlled(props),
Expand Down Expand Up @@ -171,7 +171,7 @@ export function updateWrapper(element: Element, props: Object) {

updateChecked(element, props);

const value = getSafeValue(props.value);
const value = getToStringValue(props.value);

if (value != null) {
if (props.type === 'number') {
Expand All @@ -181,17 +181,17 @@ export function updateWrapper(element: Element, props: Object) {
// eslint-disable-next-line
node.value != (value: any)
) {
node.value = safeValueToString(value);
node.value = toString(value);
}
} else if (node.value !== safeValueToString(value)) {
node.value = safeValueToString(value);
} else if (node.value !== toString(value)) {
node.value = toString(value);
}
}

if (props.hasOwnProperty('value')) {
setDefaultValue(node, props.type, value);
} else if (props.hasOwnProperty('defaultValue')) {
setDefaultValue(node, props.type, getSafeValue(props.defaultValue));
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
}

if (props.checked == null && props.defaultChecked != null) {
Expand All @@ -207,7 +207,7 @@ export function postMountWrapper(
const node = ((element: any): InputWithWrapperState);

if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
const initialValue = safeValueToString(node._wrapperState.initialValue);
const initialValue = toString(node._wrapperState.initialValue);
const currentValue = node.value;

// Do not assign value if it is already set. This prevents user text input
Expand Down Expand Up @@ -316,9 +316,9 @@ export function setDefaultValue(
node.ownerDocument.activeElement !== node
) {
if (value == null) {
node.defaultValue = safeValueToString(node._wrapperState.initialValue);
} else if (node.defaultValue !== safeValueToString(value)) {
node.defaultValue = safeValueToString(value);
node.defaultValue = toString(node._wrapperState.initialValue);
} else if (node.defaultValue !== toString(value)) {
node.defaultValue = toString(value);
}
}
}
Expand Up @@ -7,16 +7,22 @@
* @flow
*/

export opaque type SafeValue = boolean | number | Object | string | null | void;
export opaque type ToStringValue =
| 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 {
// passing the value through getToStringValue first.
export function toString(value: ToStringValue): string {
return '' + (value: any);
}

export function getSafeValue(value: mixed): SafeValue {
export function getToStringValue(value: mixed): ToStringValue {
switch (typeof value) {
case 'boolean':
case 'number':
Expand Down

0 comments on commit da6b9e0

Please sign in to comment.