Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure value and defaultValue do not assign functions and symbols #11741

Merged
merged 15 commits into from
Dec 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions fixtures/attribute-behavior/AttributeTableSnapshot.md
Original file line number Diff line number Diff line change
Expand Up @@ -2593,8 +2593,8 @@
| `defaultValue=(string 'false')`| (changed)| `"false"` |
| `defaultValue=(string 'on')`| (changed)| `"on"` |
| `defaultValue=(string 'off')`| (changed)| `"off"` |
| `defaultValue=(symbol)`| (changed, error, warning, ssr error)| `` |
| `defaultValue=(function)`| (changed, ssr warning)| `"function f() {}"` |
| `defaultValue=(symbol)`| (initial, ssr error, ssr mismatch)| `<empty string>` |
| `defaultValue=(function)`| (initial, ssr mismatch)| `<empty string>` |
| `defaultValue=(null)`| (initial, ssr warning)| `<empty string>` |
| `defaultValue=(undefined)`| (initial)| `<empty string>` |

Expand Down Expand Up @@ -11768,8 +11768,8 @@
| `value=(string 'false')`| (changed)| `"false"` |
| `value=(string 'on')`| (changed)| `"on"` |
| `value=(string 'off')`| (changed)| `"off"` |
| `value=(symbol)`| (changed, error, warning, ssr error)| `` |
| `value=(function)`| (changed, warning, ssr warning)| `"function f() {}"` |
| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `<empty string>` |
| `value=(function)`| (initial, warning, ssr mismatch)| `<empty string>` |
| `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
| `value=(undefined)`| (initial)| `<empty string>` |

Expand All @@ -11793,8 +11793,8 @@
| `value=(string 'false')`| (changed)| `"false"` |
| `value=(string 'on')`| (changed)| `"on"` |
| `value=(string 'off')`| (changed)| `"off"` |
| `value=(symbol)`| (changed, error, warning, ssr error)| `` |
| `value=(function)`| (changed, warning)| `"function f() {}"` |
| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `<empty string>` |
| `value=(function)`| (initial, warning, ssr mismatch)| `<empty string>` |
| `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
| `value=(undefined)`| (initial)| `<empty string>` |

Expand All @@ -11818,7 +11818,7 @@
| `value=(string 'false')`| (initial)| `<empty string>` |
| `value=(string 'on')`| (initial)| `<empty string>` |
| `value=(string 'off')`| (initial)| `<empty string>` |
| `value=(symbol)`| (changed, error, warning, ssr error)| `` |
| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `<empty string>` |
| `value=(function)`| (initial, warning)| `<empty string>` |
| `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
| `value=(undefined)`| (initial)| `<empty string>` |
Expand Down
183 changes: 183 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,23 @@ describe('ReactDOMInput', () => {
}
});

it('performs a state change from "" to 0', () => {
class Stub extends React.Component {
state = {
value: '',
};
render() {
return <input type="number" value={this.state.value} readOnly={true} />;
}
}

var stub = ReactTestUtils.renderIntoDocument(<Stub />);
var node = ReactDOM.findDOMNode(stub);
stub.setState({value: 0});

expect(node.value).toEqual('0');
});

it('distinguishes precision for extra zeroes in string number values', () => {
spyOnDev(console, 'error');
class Stub extends React.Component {
Expand Down Expand Up @@ -595,6 +612,7 @@ describe('ReactDOMInput', () => {
var node = container.firstChild;

expect(node.value).toBe('0');
expect(node.defaultValue).toBe('0');
});

it('should properly transition from 0 to an empty value', function() {
Expand All @@ -606,6 +624,43 @@ describe('ReactDOMInput', () => {
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.defaultValue).toBe('');
});

it('should properly transition a text input from 0 to an empty 0.0', function() {
var container = document.createElement('div');

ReactDOM.render(<input type="text" value={0} />, container);
ReactDOM.render(<input type="text" value="0.0" />, container);

var node = container.firstChild;

expect(node.value).toBe('0.0');
expect(node.defaultValue).toBe('0.0');
});

it('should properly transition a number input from "" to 0', function() {
var container = document.createElement('div');

ReactDOM.render(<input type="number" value="" />, container);
ReactDOM.render(<input type="number" value={0} />, container);

var node = container.firstChild;

expect(node.value).toBe('0');
expect(node.defaultValue).toBe('0');
});

it('should properly transition a number input from "" to "0"', function() {
var container = document.createElement('div');

ReactDOM.render(<input type="number" value="" />, container);
ReactDOM.render(<input type="number" value="0" />, container);

var node = container.firstChild;

expect(node.value).toBe('0');
expect(node.defaultValue).toBe('0');
});

it('should have the correct target value', () => {
Expand Down Expand Up @@ -1585,4 +1640,132 @@ describe('ReactDOMInput', () => {
}
});
});

describe('When given a Symbol value', function() {
it('treats initial Symbol value as an empty string', function() {
spyOnDev(console, 'error');
var container = document.createElement('div');
ReactDOM.render(
<input value={Symbol('foobar')} onChange={() => {}} />,
container,
);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Invalid value for prop `value`',
);
}
});

it('treats updated Symbol value as an empty string', function() {
spyOnDev(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<input value="foo" onChange={() => {}} />, container);
ReactDOM.render(
<input value={Symbol('foobar')} onChange={() => {}} />,
container,
);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Invalid value for prop `value`',
);
}
});

it('treats initial Symbol defaultValue as an empty string', function() {
var container = document.createElement('div');
ReactDOM.render(<input defaultValue={Symbol('foobar')} />, container);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');
// TODO: we should warn here.
});

it('treats updated Symbol defaultValue as an empty string', function() {
var container = document.createElement('div');
ReactDOM.render(<input defaultValue="foo" />, container);
ReactDOM.render(<input defaultValue={Symbol('foobar')} />, container);
var node = container.firstChild;

expect(node.value).toBe('foo');
expect(node.getAttribute('value')).toBe('');
// TODO: we should warn here.
});
});

describe('When given a function value', function() {
it('treats initial function value as an empty string', function() {
spyOnDev(console, 'error');
var container = document.createElement('div');
ReactDOM.render(
<input value={() => {}} onChange={() => {}} />,
container,
);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Invalid value for prop `value`',
);
}
});

it('treats updated function value as an empty string', function() {
spyOnDev(console, 'error');
var container = document.createElement('div');
ReactDOM.render(<input value="foo" onChange={() => {}} />, container);
ReactDOM.render(
<input value={() => {}} onChange={() => {}} />,
container,
);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Invalid value for prop `value`',
);
}
});

it('treats initial function defaultValue as an empty string', function() {
var container = document.createElement('div');
ReactDOM.render(<input defaultValue={() => {}} />, container);
var node = container.firstChild;

expect(node.value).toBe('');
expect(node.getAttribute('value')).toBe('');
// TODO: we should warn here.
});

it('treats updated function defaultValue as an empty string', function() {
var container = document.createElement('div');
ReactDOM.render(<input defaultValue="foo" />, container);
ReactDOM.render(<input defaultValue={() => {}} />, container);
var node = container.firstChild;

expect(node.value).toBe('foo');
expect(node.getAttribute('value')).toBe('');
// TODO: we should warn here.
});
});
});
72 changes: 39 additions & 33 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,15 @@ export function initWrapperState(element: Element, props: Object) {
}
}

var defaultValue = props.defaultValue == null ? '' : props.defaultValue;
var node = ((element: any): InputWithWrapperState);
var defaultValue = props.defaultValue == null ? '' : props.defaultValue;

node._wrapperState = {
initialChecked:
props.checked != null ? props.checked : props.defaultChecked,
initialValue: props.value != null ? props.value : defaultValue,
initialValue: getSafeValue(
props.value != null ? props.value : defaultValue,
),
controlled: isControlled(props),
};
}
Expand Down Expand Up @@ -175,36 +178,26 @@ export function updateWrapper(element: Element, props: Object) {

updateChecked(element, props);

var value = props.value;
if (value != null) {
if (value === 0 && node.value === '') {
node.value = '0';
// Note: IE9 reports a number inputs as 'text', so check props instead.
} else if (props.type === 'number') {
// Simulate `input.valueAsNumber`. IE9 does not support it
var valueAsNumber = parseFloat(node.value) || 0;
var value = getSafeValue(props.value);

if (value != null) {
if (props.type === 'number') {
if (
(value === 0 && node.value === '') ||
// eslint-disable-next-line
value != valueAsNumber ||
// eslint-disable-next-line
(value == valueAsNumber && node.value != value)
node.value != value
) {
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
}
} else if (node.value !== '' + value) {
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
}
synchronizeDefaultValue(node, props.type, value);
} else if (
props.hasOwnProperty('value') ||
props.hasOwnProperty('defaultValue')
) {
synchronizeDefaultValue(node, props.type, props.defaultValue);
}

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

if (props.checked == null && props.defaultChecked != null) {
Expand All @@ -214,19 +207,18 @@ export function updateWrapper(element: Element, props: Object) {

export function postMountWrapper(element: Element, props: Object) {
var node = ((element: any): InputWithWrapperState);
var initialValue = node._wrapperState.initialValue;

if (props.value != null || props.defaultValue != null) {
if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
// Do not assign value if it is already set. This prevents user text input
// from being lost during SSR hydration.
if (node.value === '') {
node.value = initialValue;
node.value = '' + node._wrapperState.initialValue;
}

// value must be assigned before defaultValue. This fixes an issue where the
// visually displayed value of date inputs disappears on mobile Safari and Chrome:
// https://github.com/facebook/react/issues/7233
node.defaultValue = initialValue;
node.defaultValue = '' + node._wrapperState.initialValue;
}

// Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
Expand Down Expand Up @@ -307,20 +299,34 @@ function updateNamedCousins(rootNode, props) {
// when the user is inputting text
//
// https://github.com/facebook/react/issues/7253
export function synchronizeDefaultValue(
export function setDefaultValue(
node: InputWithWrapperState,
type: ?string,
value: *,
) {
if (
// Focused number inputs synchronize on blur. See ChangeEventPlugin.js
(type !== 'number' || node.ownerDocument.activeElement !== node) &&
node.defaultValue !== '' + value
type !== 'number' ||
node.ownerDocument.activeElement !== node
) {
if (value != null) {
if (value == null) {
node.defaultValue = '' + node._wrapperState.initialValue;
} else if (node.defaultValue !== '' + value) {
node.defaultValue = '' + value;
} else {
node.defaultValue = node._wrapperState.initialValue;
}
}
}

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 '';
}
}
4 changes: 2 additions & 2 deletions packages/react-dom/src/events/ChangeEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import getEventTarget from './getEventTarget';
import isEventSupported from './isEventSupported';
import {getNodeFromInstance} from '../client/ReactDOMComponentTree';
import * as inputValueTracking from '../client/inputValueTracking';
import {synchronizeDefaultValue} from '../client/ReactDOMFiberInput';
import {setDefaultValue} from '../client/ReactDOMFiberInput';

var eventTypes = {
change: {
Expand Down Expand Up @@ -236,7 +236,7 @@ function handleControlledInputBlur(inst, node) {
}

// If controlled, assign the value attribute to the current value on blur
synchronizeDefaultValue(node, 'number', node.value);
setDefaultValue(node, 'number', node.value);
}

/**
Expand Down
Loading