Skip to content

Commit

Permalink
🔥 Stop syncing the value attribute on inputs
Browse files Browse the repository at this point in the history
  • Loading branch information
nhunzaker committed Aug 31, 2018
1 parent 0452c9b commit 10473d0
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 35 deletions.
39 changes: 12 additions & 27 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ describe('ReactDOMInput', () => {

dispatchEventOnNode(node, 'input');

expect(node.getAttribute('value')).toBe('2.0');
expect(node.getAttribute('value')).toBe('2');
expect(node.value).toBe('2.0');
});
});
Expand Down Expand Up @@ -667,7 +667,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('should properly transition from 0 to an empty value', function() {
Expand All @@ -683,7 +683,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('should properly transition a text input from 0 to an empty 0.0', function() {
Expand All @@ -699,7 +699,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('should properly transition a number input from "" to 0', function() {
Expand All @@ -715,7 +715,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('should properly transition a number input from "" to "0"', function() {
Expand All @@ -731,7 +731,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

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

it('always sets the attribute when values change on text inputs', function() {
it('retains the initial value attribute when values change on text inputs', function() {
const Input = getTestInput();
const stub = ReactDOM.render(<Input type="text" />, container);
const node = ReactDOM.findDOMNode(stub);

setUntrackedValue.call(node, '2');
dispatchEventOnNode(node, 'input');

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

it('does not set the value attribute on number inputs if focused', () => {
Expand All @@ -1632,21 +1632,6 @@ describe('ReactDOMInput', () => {
expect(node.getAttribute('value')).toBe('1');
});

it('sets the value attribute on number inputs on blur', () => {
const Input = getTestInput();
const stub = ReactDOM.render(
<Input type="number" value="1" />,
container,
);
const node = ReactDOM.findDOMNode(stub);

setUntrackedValue.call(node, '2');
dispatchEventOnNode(node, 'input');
dispatchEventOnNode(node, 'blur');

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

it('an uncontrolled number input will not update the value attribute on blur', () => {
const node = ReactDOM.render(
<input type="number" defaultValue="1" />,
Expand Down Expand Up @@ -1784,7 +1769,7 @@ describe('ReactDOMInput', () => {
expect(node.getAttribute('value')).toBe('');
});

it('treats updated Symbol value as an empty string', function() {
it('treats updated Symbol value as initial value', function() {
ReactDOM.render(<input value="foo" onChange={() => {}} />, container);
expect(() =>
ReactDOM.render(
Expand All @@ -1795,7 +1780,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('treats initial Symbol defaultValue as an empty string', function() {
Expand Down Expand Up @@ -1832,7 +1817,7 @@ describe('ReactDOMInput', () => {
expect(node.getAttribute('value')).toBe('');
});

it('treats updated function value as an empty string', function() {
it('treats updated function value as initial value', function() {
ReactDOM.render(<input value="foo" onChange={() => {}} />, container);
expect(() =>
ReactDOM.render(
Expand All @@ -1843,7 +1828,7 @@ describe('ReactDOMInput', () => {
const node = container.firstChild;

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

it('treats initial function defaultValue as an empty string', function() {
Expand Down
6 changes: 2 additions & 4 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ export function updateWrapper(element: Element, props: Object) {
return;
}

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

Expand Down Expand Up @@ -321,7 +319,7 @@ function updateNamedCousins(rootNode, props) {
// when the user is inputting text
//
// https:/facebook/react/issues/7253
export function setDefaultValue(
function setDefaultValue(
node: InputWithWrapperState,
type: ?string,
value: *,
Expand Down
4 changes: 0 additions & 4 deletions packages/react-dom/src/events/ChangeEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import getEventTarget from './getEventTarget';
import isEventSupported from './isEventSupported';
import {getNodeFromInstance} from '../client/ReactDOMComponentTree';
import * as inputValueTracking from '../client/inputValueTracking';
import {setDefaultValue} from '../client/ReactDOMFiberInput';

const eventTypes = {
change: {
Expand Down Expand Up @@ -237,9 +236,6 @@ function handleControlledInputBlur(node) {
if (!state || !state.controlled || node.type !== 'number') {
return;
}

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

/**
Expand Down

0 comments on commit 10473d0

Please sign in to comment.