diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
index 098285224c171..e37c61a35dc97 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -834,6 +834,7 @@ export function setInitialProperties(
// listeners still fire for the invalid event.
listenToNonDelegatedEvent('invalid', domElement);
+ let name = null;
let type = null;
let value = null;
let defaultValue = null;
@@ -848,31 +849,16 @@ export function setInitialProperties(
continue;
}
switch (propKey) {
+ case 'name': {
+ name = propValue;
+ break;
+ }
case 'type': {
- // Fast path since 'type' is very common on inputs
- if (
- propValue != null &&
- typeof propValue !== 'function' &&
- typeof propValue !== 'symbol' &&
- typeof propValue !== 'boolean'
- ) {
- type = propValue;
- if (__DEV__) {
- checkAttributeStringCoercion(propValue, propKey);
- }
- domElement.setAttribute(propKey, propValue);
- }
+ type = propValue;
break;
}
case 'checked': {
checked = propValue;
- const checkedValue =
- propValue != null ? propValue : props.defaultChecked;
- const inputElement: HTMLInputElement = (domElement: any);
- inputElement.checked =
- !!checkedValue &&
- typeof checkedValue !== 'function' &&
- checkedValue !== 'symbol';
break;
}
case 'defaultChecked': {
@@ -904,7 +890,6 @@ export function setInitialProperties(
}
// TODO: Make sure we check if this is still unmounted or do any clean
// up necessary since we never stop tracking anymore.
- track((domElement: any));
validateInputProps(domElement, props);
initInput(
domElement,
@@ -913,8 +898,10 @@ export function setInitialProperties(
checked,
defaultChecked,
type,
+ name,
false,
);
+ track((domElement: any));
return;
}
case 'select': {
@@ -1010,9 +997,9 @@ export function setInitialProperties(
}
// TODO: Make sure we check if this is still unmounted or do any clean
// up necessary since we never stop tracking anymore.
- track((domElement: any));
validateTextareaProps(domElement, props);
initTextarea(domElement, value, defaultValue, children);
+ track((domElement: any));
return;
}
case 'option': {
@@ -1305,14 +1292,6 @@ export function updateProperties(
if (lastProps.hasOwnProperty(propKey) && lastProp != null) {
switch (propKey) {
case 'checked': {
- if (!nextProps.hasOwnProperty(propKey)) {
- const checkedValue = nextProps.defaultChecked;
- const inputElement: HTMLInputElement = (domElement: any);
- inputElement.checked =
- !!checkedValue &&
- typeof checkedValue !== 'function' &&
- checkedValue !== 'symbol';
- }
break;
}
case 'value': {
@@ -1341,22 +1320,6 @@ export function updateProperties(
switch (propKey) {
case 'type': {
type = nextProp;
- // Fast path since 'type' is very common on inputs
- if (nextProp !== lastProp) {
- if (
- nextProp != null &&
- typeof nextProp !== 'function' &&
- typeof nextProp !== 'symbol' &&
- typeof nextProp !== 'boolean'
- ) {
- if (__DEV__) {
- checkAttributeStringCoercion(nextProp, propKey);
- }
- domElement.setAttribute(propKey, nextProp);
- } else {
- domElement.removeAttribute(propKey);
- }
- }
break;
}
case 'name': {
@@ -1365,15 +1328,6 @@ export function updateProperties(
}
case 'checked': {
checked = nextProp;
- if (nextProp !== lastProp) {
- const checkedValue =
- nextProp != null ? nextProp : nextProps.defaultChecked;
- const inputElement: HTMLInputElement = (domElement: any);
- inputElement.checked =
- !!checkedValue &&
- typeof checkedValue !== 'function' &&
- checkedValue !== 'symbol';
- }
break;
}
case 'defaultChecked': {
@@ -1453,23 +1407,6 @@ export function updateProperties(
}
}
- // Update checked *before* name.
- // In the middle of an update, it is possible to have multiple checked.
- // When a checked radio tries to change name, browser makes another radio's checked false.
- if (
- name != null &&
- typeof name !== 'function' &&
- typeof name !== 'symbol' &&
- typeof name !== 'boolean'
- ) {
- if (__DEV__) {
- checkAttributeStringCoercion(name, 'name');
- }
- domElement.setAttribute('name', name);
- } else {
- domElement.removeAttribute('name');
- }
-
// Update the wrapper around inputs *after* updating props. This has to
// happen after updating the rest of props. Otherwise HTML5 input validations
// raise warnings and prevent the new value from being assigned.
@@ -1481,6 +1418,7 @@ export function updateProperties(
checked,
defaultChecked,
type,
+ name,
);
return;
}
@@ -1822,33 +1760,12 @@ export function updatePropertiesWithDiff(
const propValue = updatePayload[i + 1];
switch (propKey) {
case 'type': {
- // Fast path since 'type' is very common on inputs
- if (
- propValue != null &&
- typeof propValue !== 'function' &&
- typeof propValue !== 'symbol' &&
- typeof propValue !== 'boolean'
- ) {
- if (__DEV__) {
- checkAttributeStringCoercion(propValue, propKey);
- }
- domElement.setAttribute(propKey, propValue);
- } else {
- domElement.removeAttribute(propKey);
- }
break;
}
case 'name': {
break;
}
case 'checked': {
- const checkedValue =
- propValue != null ? propValue : nextProps.defaultChecked;
- const inputElement: HTMLInputElement = (domElement: any);
- inputElement.checked =
- !!checkedValue &&
- typeof checkedValue !== 'function' &&
- checkedValue !== 'symbol';
break;
}
case 'defaultChecked': {
@@ -1916,23 +1833,6 @@ export function updatePropertiesWithDiff(
}
}
- // Update checked *before* name.
- // In the middle of an update, it is possible to have multiple checked.
- // When a checked radio tries to change name, browser makes another radio's checked false.
- if (
- name != null &&
- typeof name !== 'function' &&
- typeof name !== 'symbol' &&
- typeof name !== 'boolean'
- ) {
- if (__DEV__) {
- checkAttributeStringCoercion(name, 'name');
- }
- domElement.setAttribute('name', name);
- } else {
- domElement.removeAttribute('name');
- }
-
// Update the wrapper around inputs *after* updating props. This has to
// happen after updating the rest of props. Otherwise HTML5 input validations
// raise warnings and prevent the new value from being assigned.
@@ -1944,6 +1844,7 @@ export function updatePropertiesWithDiff(
checked,
defaultChecked,
type,
+ name,
);
return;
}
@@ -2970,7 +2871,6 @@ export function diffHydratedProperties(
listenToNonDelegatedEvent('invalid', domElement);
// TODO: Make sure we check if this is still unmounted or do any clean
// up necessary since we never stop tracking anymore.
- track((domElement: any));
validateInputProps(domElement, props);
// For input and textarea we current always set the value property at
// post mount to force it to diverge from attributes. However, for
@@ -2984,8 +2884,10 @@ export function diffHydratedProperties(
props.checked,
props.defaultChecked,
props.type,
+ props.name,
true,
);
+ track((domElement: any));
break;
case 'option':
validateOptionProps(domElement, props);
@@ -3008,9 +2910,9 @@ export function diffHydratedProperties(
listenToNonDelegatedEvent('invalid', domElement);
// TODO: Make sure we check if this is still unmounted or do any clean
// up necessary since we never stop tracking anymore.
- track((domElement: any));
validateTextareaProps(domElement, props);
initTextarea(domElement, props.value, props.defaultValue, props.children);
+ track((domElement: any));
break;
}
diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js
index 4e8f9b32aa7e3..2096238d762f6 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMInput.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js
@@ -89,9 +89,30 @@ export function updateInput(
checked: ?boolean,
defaultChecked: ?boolean,
type: ?string,
+ name: ?string,
) {
const node: HTMLInputElement = (element: any);
+ // Temporarily disconnect the input from any radio buttons.
+ // Changing the type or name as the same time as changing the checked value
+ // needs to be atomically applied. We can only ensure that by disconnecting
+ // the name while do the mutations and then reapply the name after that's done.
+ node.name = '';
+
+ if (
+ type != null &&
+ typeof type !== 'function' &&
+ typeof type !== 'symbol' &&
+ typeof type !== 'boolean'
+ ) {
+ if (__DEV__) {
+ checkAttributeStringCoercion(type, 'type');
+ }
+ node.type = type;
+ } else {
+ node.removeAttribute('type');
+ }
+
if (value != null) {
if (type === 'number') {
if (
@@ -157,6 +178,20 @@ export function updateInput(
if (checked != null && node.checked !== !!checked) {
node.checked = checked;
}
+
+ if (
+ name != null &&
+ typeof name !== 'function' &&
+ typeof name !== 'symbol' &&
+ typeof name !== 'boolean'
+ ) {
+ if (__DEV__) {
+ checkAttributeStringCoercion(name, 'name');
+ }
+ node.name = name;
+ } else {
+ node.removeAttribute('name');
+ }
}
export function initInput(
@@ -166,10 +201,23 @@ export function initInput(
checked: ?boolean,
defaultChecked: ?boolean,
type: ?string,
+ name: ?string,
isHydrating: boolean,
) {
const node: HTMLInputElement = (element: any);
+ if (
+ type != null &&
+ typeof type !== 'function' &&
+ typeof type !== 'symbol' &&
+ typeof type !== 'boolean'
+ ) {
+ if (__DEV__) {
+ checkAttributeStringCoercion(type, 'type');
+ }
+ node.type = type;
+ }
+
if (value != null || defaultValue != null) {
const isButton = type === 'submit' || type === 'reset';
@@ -235,10 +283,6 @@ export function initInput(
// will sometimes influence the value of checked (even after detachment).
// Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=608416
// We need to temporarily unset name to avoid disrupting radio button groups.
- const name = node.name;
- if (name !== '') {
- node.name = '';
- }
const checkedOrDefault = checked != null ? checked : defaultChecked;
// TODO: This 'function' or 'symbol' check isn't replicated in other places
@@ -276,7 +320,16 @@ export function initInput(
node.defaultChecked = !!initialChecked;
}
- if (name !== '') {
+ // Name needs to be set at the end so that it applies atomically to connected radio buttons.
+ if (
+ name != null &&
+ typeof name !== 'function' &&
+ typeof name !== 'symbol' &&
+ typeof name !== 'boolean'
+ ) {
+ if (__DEV__) {
+ checkAttributeStringCoercion(name, 'name');
+ }
node.name = name;
}
}
@@ -291,6 +344,7 @@ export function restoreControlledInputState(element: Element, props: Object) {
props.checked,
props.defaultChecked,
props.type,
+ props.name,
);
const name = props.name;
if (props.type === 'radio' && name != null) {
@@ -347,6 +401,7 @@ export function restoreControlledInputState(element: Element, props: Object) {
otherProps.checked,
otherProps.defaultChecked,
otherProps.type,
+ otherProps.name,
);
}
}
diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
index a195bd67bce52..f5493c059fc4d 100644
--- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -1143,7 +1143,8 @@ describe('ReactDOMComponent', () => {
'the value changing from a defined to undefined, which should not happen. Decide between ' +
'using a controlled or uncontrolled input element for the lifetime of the component.',
);
- expect(nodeValueSetter).toHaveBeenCalledTimes(1);
+ // This leaves the current checked value in place, just like text inputs.
+ expect(nodeValueSetter).toHaveBeenCalledTimes(0);
expect(() => {
ReactDOM.render(
@@ -1156,13 +1157,13 @@ describe('ReactDOMComponent', () => {
'using a controlled or uncontrolled input element for the lifetime of the component.',
);
- expect(nodeValueSetter).toHaveBeenCalledTimes(2);
+ expect(nodeValueSetter).toHaveBeenCalledTimes(1);
ReactDOM.render(
,
container,
);
- expect(nodeValueSetter).toHaveBeenCalledTimes(3);
+ expect(nodeValueSetter).toHaveBeenCalledTimes(2);
});
it('should ignore attribute list for elements with the "is" attribute', () => {
diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js
index 0c736174e4545..0f02d928c770b 100644
--- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js
@@ -1191,7 +1191,7 @@ describe('ReactDOMInput', () => {
updated: false,
};
onClick = () => {
- this.setState({updated: true});
+ this.setState({updated: !this.state.updated});
};
render() {
const {updated} = this.state;
@@ -1222,6 +1222,62 @@ describe('ReactDOMInput', () => {
expect(firstRadioNode.checked).toBe(false);
dispatchEventOnNode(buttonNode, 'click');
expect(firstRadioNode.checked).toBe(true);
+ dispatchEventOnNode(buttonNode, 'click');
+ expect(firstRadioNode.checked).toBe(false);
+ });
+
+ it("shouldn't get tricked by changing radio names, part 2", () => {
+ ReactDOM.render(
+
,
+ container,
+ );
+ expect(container.querySelector('input[name="a"][value="1"]').checked).toBe(
+ true,
+ );
+ expect(container.querySelector('input[name="b"][value="2"]').checked).toBe(
+ true,
+ );
});
it('should control radio buttons if the tree updates during render', () => {
@@ -1720,8 +1776,18 @@ describe('ReactDOMInput', () => {
) {
const el = originalCreateElement.apply(this, arguments);
let value = '';
+ let typeProp = '';
if (type === 'input') {
+ Object.defineProperty(el, 'type', {
+ get: function () {
+ return typeProp;
+ },
+ set: function (val) {
+ typeProp = String(val);
+ log.push('set property type');
+ },
+ });
Object.defineProperty(el, 'value', {
get: function () {
return value;
@@ -1751,10 +1817,10 @@ describe('ReactDOMInput', () => {
);
expect(log).toEqual([
- 'set attribute type',
'set attribute min',
'set attribute max',
'set attribute step',
+ 'set property type',
'set property value',
]);
});
@@ -1810,6 +1876,14 @@ describe('ReactDOMInput', () => {
HTMLInputElement.prototype,
'value',
).set;
+ const getType = Object.getOwnPropertyDescriptor(
+ HTMLInputElement.prototype,
+ 'type',
+ ).get;
+ const setType = Object.getOwnPropertyDescriptor(
+ HTMLInputElement.prototype,
+ 'type',
+ ).set;
if (type === 'input') {
Object.defineProperty(el, 'defaultValue', {
get: function () {
@@ -1829,6 +1903,15 @@ describe('ReactDOMInput', () => {
setValue.call(this, val);
},
});
+ Object.defineProperty(el, 'type', {
+ get: function () {
+ return getType.call(this);
+ },
+ set: function (val) {
+ log.push(`node.type = ${strify(val)}`);
+ setType.call(this, val);
+ },
+ });
spyOnDevAndProd(el, 'setAttribute').mockImplementation(function (
name,
val,
@@ -1843,14 +1926,14 @@ describe('ReactDOMInput', () => {
if (disableInputAttributeSyncing) {
expect(log).toEqual([
- 'node.setAttribute("type", "date")',
+ 'node.type = "date"',
'node.defaultValue = "1980-01-01"',
// TODO: it's possible this reintroduces the bug because we don't assign `value` at all.
// Need to check this on mobile Safari and Chrome.
]);
} else {
expect(log).toEqual([
- 'node.setAttribute("type", "date")',
+ 'node.type = "date"',
// 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
diff --git a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
index f4f75bfd52233..ae27973a0c603 100644
--- a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
+++ b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
@@ -12,7 +12,6 @@
let React;
let ReactDOM;
let ReactDOMClient;
-let ReactFeatureFlags;
let Scheduler;
let act;
let waitForAll;
@@ -39,7 +38,6 @@ describe('ChangeEventPlugin', () => {
beforeEach(() => {
jest.resetModules();
- ReactFeatureFlags = require('shared/ReactFeatureFlags');
// TODO pull this into helper method, reduce repetition.
// mock the browser APIs which are used in schedule:
// - calling 'window.postMessage' should actually fire postmessage handlers
@@ -100,13 +98,8 @@ describe('ChangeEventPlugin', () => {
node.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));
- if (ReactFeatureFlags.disableInputAttributeSyncing) {
- // TODO: figure out why. This might be a bug.
- expect(called).toBe(1);
- } else {
- // There should be no React change events because the value stayed the same.
- expect(called).toBe(0);
- }
+ // There should be no React change events because the value stayed the same.
+ expect(called).toBe(0);
});
it('should consider initial text value to be current (capture)', () => {
@@ -124,13 +117,8 @@ describe('ChangeEventPlugin', () => {
node.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));
- if (ReactFeatureFlags.disableInputAttributeSyncing) {
- // TODO: figure out why. This might be a bug.
- expect(called).toBe(1);
- } else {
- // There should be no React change events because the value stayed the same.
- expect(called).toBe(0);
- }
+ // There should be no React change events because the value stayed the same.
+ expect(called).toBe(0);
});
it('should not invoke a change event for textarea same value', () => {