From cf9d8bfedf339415f75c79f48ff66a399597a713 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 6 Apr 2023 21:58:20 -0400 Subject: [PATCH 1/3] Use already extracted values instead of reading off props for inptut --- .../src/client/ReactDOMComponent.js | 233 ++++++++++++++---- .../src/client/ReactDOMInput.js | 122 ++++----- 2 files changed, 253 insertions(+), 102 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index e2ba29ceb3aed..b9d2a1898399a 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -28,7 +28,6 @@ import { import { validateInputProps, initInput, - updateInputChecked, updateInput, restoreControlledInputState, } from './ReactDOMInput'; @@ -834,6 +833,12 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); + + let type = null; + let value = null; + let defaultValue = null; + let checked = null; + let defaultChecked = null; for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; @@ -851,6 +856,7 @@ export function setInitialProperties( typeof propValue !== 'symbol' && typeof propValue !== 'boolean' ) { + type = propValue; if (__DEV__) { checkAttributeStringCoercion(propValue, propKey); } @@ -859,17 +865,26 @@ export function setInitialProperties( break; } case 'checked': { - const checked = + checked = propValue; + const checkedValue = propValue != null ? propValue : props.defaultChecked; const inputElement: HTMLInputElement = (domElement: any); inputElement.checked = - !!checked && - typeof checked !== 'function' && - checked !== 'symbol'; + !!checkedValue && + typeof checkedValue !== 'function' && + checkedValue !== 'symbol'; + break; + } + case 'defaultChecked': { + defaultChecked = propValue; break; } case 'value': { - // This is handled by updateWrapper below. + value = propValue; + break; + } + case 'defaultValue': { + defaultValue = propValue; break; } case 'children': @@ -882,7 +897,6 @@ export function setInitialProperties( } break; } - // defaultChecked and defaultValue are ignored by setProp default: { setProp(domElement, tag, propKey, propValue, props, null); } @@ -892,7 +906,15 @@ export function setInitialProperties( // up necessary since we never stop tracking anymore. track((domElement: any)); validateInputProps(domElement, props); - initInput(domElement, props, false); + initInput( + domElement, + value, + defaultValue, + checked, + defaultChecked, + type, + false, + ); return; } case 'select': { @@ -1250,12 +1272,12 @@ export function updateProperties( break; } case 'input': { - // 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 (nextProps.type === 'radio' && nextProps.name != null) { - updateInputChecked(domElement, nextProps); - } + let name = null; + let type = null; + let value = null; + let defaultValue = null; + let checked = null; + let defaultChecked = null; for (const propKey in lastProps) { const lastProp = lastProps[propKey]; if ( @@ -1265,12 +1287,12 @@ export function updateProperties( ) { switch (propKey) { case 'checked': { - const checked = nextProps.defaultChecked; + const checkedValue = nextProps.defaultChecked; const inputElement: HTMLInputElement = (domElement: any); inputElement.checked = - !!checked && - typeof checked !== 'function' && - checked !== 'symbol'; + !!checkedValue && + typeof checkedValue !== 'function' && + checkedValue !== 'symbol'; break; } case 'value': { @@ -1289,22 +1311,56 @@ export function updateProperties( const lastProp = lastProps[propKey]; if ( nextProps.hasOwnProperty(propKey) && - nextProp !== lastProp && (nextProp != null || lastProp != null) ) { 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': { + name = nextProp; + break; + } case 'checked': { - const checked = - nextProp != null ? nextProp : nextProps.defaultChecked; - const inputElement: HTMLInputElement = (domElement: any); - inputElement.checked = - !!checked && - typeof checked !== 'function' && - checked !== 'symbol'; + 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': { + defaultChecked = nextProp; break; } case 'value': { - // This is handled by updateWrapper below. + value = nextProp; + break; + } + case 'defaultValue': { + defaultValue = nextProp; break; } case 'children': @@ -1317,9 +1373,16 @@ export function updateProperties( } break; } - // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, nextProp, nextProps, lastProp); + if (nextProp !== lastProp) + setProp( + domElement, + tag, + propKey, + nextProp, + nextProps, + lastProp, + ); } } } @@ -1364,10 +1427,35 @@ export function updateProperties( didWarnControlledToUncontrolled = true; } } + + // 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. - updateInput(domElement, nextProps); + updateInput( + domElement, + value, + defaultValue, + checked, + defaultChecked, + type, + ); return; } case 'select': { @@ -1661,28 +1749,53 @@ export function updatePropertiesWithDiff( break; } case 'input': { - // 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 (nextProps.type === 'radio' && nextProps.name != null) { - updateInputChecked(domElement, nextProps); - } + const name = nextProps.name; + const type = nextProps.type; + const value = nextProps.value; + const defaultValue = nextProps.defaultValue; + const checked = nextProps.checked; + const defaultChecked = nextProps.defaultChecked; for (let i = 0; i < updatePayload.length; i += 2) { const propKey = updatePayload[i]; 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 checked = + const checkedValue = propValue != null ? propValue : nextProps.defaultChecked; const inputElement: HTMLInputElement = (domElement: any); inputElement.checked = - !!checked && - typeof checked !== 'function' && - checked !== 'symbol'; + !!checkedValue && + typeof checkedValue !== 'function' && + checkedValue !== 'symbol'; + break; + } + case 'defaultChecked': { break; } case 'value': { - // This is handled by updateWrapper below. + break; + } + case 'defaultValue': { break; } case 'children': @@ -1695,7 +1808,6 @@ export function updatePropertiesWithDiff( } break; } - // defaultChecked and defaultValue are ignored by setProp default: { setProp(domElement, tag, propKey, propValue, nextProps, null); } @@ -1741,10 +1853,35 @@ export function updatePropertiesWithDiff( didWarnControlledToUncontrolled = true; } } + + // 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. - updateInput(domElement, nextProps); + updateInput( + domElement, + value, + defaultValue, + checked, + defaultChecked, + type, + ); return; } case 'select': { @@ -2771,7 +2908,15 @@ export function diffHydratedProperties( // option and select we don't quite do the same thing and select // is not resilient to the DOM state changing so we don't do that here. // TODO: Consider not doing this for input and textarea. - initInput(domElement, props, true); + initInput( + domElement, + props.value, + props.defaultValue, + props.checked, + props.defaultChecked, + props.type, + true, + ); break; case 'option': validateOptionProps(domElement, props); diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js index 0708b984f7c95..9bacf8e046314 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMInput.js +++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js @@ -81,26 +81,22 @@ export function validateInputProps(element: Element, props: Object) { } } -export function updateInputChecked(element: Element, props: Object) { - const node: HTMLInputElement = (element: any); - const checked = props.checked; - if (checked != null && node.checked !== !!checked) { - node.checked = checked; - } -} - -export function updateInput(element: Element, props: Object) { +export function updateInput( + element: Element, + value: ?string, + defaultValue: ?string, + checked: ?boolean, + defaultChecked: ?boolean, + type: ?string, +) { const node: HTMLInputElement = (element: any); - const value = getToStringValue(props.value); - const type = props.type; - if (disableInputAttributeSyncing) { // When not syncing the value attribute, React only assigns a new value // whenever the defaultValue React prop has changed. When not present, // React does nothing - if (props.defaultValue != null) { - setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); + if (defaultValue != null) { + setDefaultValue(node, type, getToStringValue(defaultValue)); } else { node.removeAttribute('value'); } @@ -110,10 +106,10 @@ export function updateInput(element: Element, props: Object) { // 1. The value React property // 2. The defaultValue React property // 3. Otherwise there should be no change - if (props.value != null) { - setDefaultValue(node, props.type, value); - } else if (props.defaultValue != null) { - setDefaultValue(node, props.type, getToStringValue(props.defaultValue)); + if (value != null) { + setDefaultValue(node, type, getToStringValue(value)); + } else if (defaultValue != null) { + setDefaultValue(node, type, getToStringValue(defaultValue)); } else { node.removeAttribute('value'); } @@ -123,20 +119,22 @@ export function updateInput(element: Element, props: Object) { // When not syncing the checked attribute, the attribute is directly // controllable from the defaultValue React property. It needs to be // updated as new props come in. - if (props.defaultChecked == null) { + if (defaultChecked == null) { node.removeAttribute('checked'); } else { - node.defaultChecked = !!props.defaultChecked; + node.defaultChecked = !!defaultChecked; } } else { // When syncing the checked attribute, it only changes when it needs // to be removed, such as transitioning from a checkbox into a text input - if (props.checked == null && props.defaultChecked != null) { - node.defaultChecked = !!props.defaultChecked; + if (checked == null && defaultChecked != null) { + node.defaultChecked = !!defaultChecked; } } - updateInputChecked(element, props); + if (checked != null && node.checked !== !!checked) { + node.checked = checked; + } if (value != null) { if (type === 'number') { @@ -147,10 +145,10 @@ export function updateInput(element: Element, props: Object) { // eslint-disable-next-line node.value != (value: any) ) { - node.value = toString((value: any)); + node.value = toString(getToStringValue(value)); } - } else if (node.value !== toString((value: any))) { - node.value = toString((value: any)); + } else if (node.value !== toString(getToStringValue(value))) { + node.value = toString(getToStringValue(value)); } } else if (type === 'submit' || type === 'reset') { // Submit/reset inputs need the attribute removed completely to avoid @@ -162,36 +160,33 @@ export function updateInput(element: Element, props: Object) { export function initInput( element: Element, - props: Object, + value: ?string, + defaultValue: ?string, + checked: ?boolean, + defaultChecked: ?boolean, + type: ?string, isHydrating: boolean, ) { const node: HTMLInputElement = (element: any); - if (props.value != null || props.defaultValue != null) { - const type = props.type; + if (value != null || defaultValue != null) { const isButton = type === 'submit' || type === 'reset'; // Avoid setting value attribute on submit/reset inputs as it overrides the // default value provided by the browser. See: #12872 - if (isButton && (props.value === undefined || props.value === null)) { + if (isButton && (value === undefined || value === null)) { return; } - const defaultValue = - props.defaultValue != null - ? toString(getToStringValue(props.defaultValue)) - : ''; + const defaultValueStr = + defaultValue != null ? toString(getToStringValue(defaultValue)) : ''; const initialValue = - props.value != null - ? toString(getToStringValue(props.value)) - : defaultValue; + value != null ? toString(getToStringValue(value)) : defaultValueStr; // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. if (!isHydrating) { if (disableInputAttributeSyncing) { - const value = getToStringValue(props.value); - // When not syncing the value attribute, the value property points // directly to the React prop. Only assign it if it exists. if (value != null) { @@ -203,8 +198,8 @@ export function initInput( // prematurely marking required inputs as invalid. Equality is compared // to the current value in case the browser provided value is not an // empty string. - if (isButton || value !== node.value) { - node.value = toString(value); + if (isButton || toString(getToStringValue(value)) !== node.value) { + node.value = toString(getToStringValue(value)); } } } else { @@ -223,8 +218,8 @@ export function initInput( if (disableInputAttributeSyncing) { // When not syncing the value attribute, assign the value attribute // directly from the defaultValue React property (when present) - if (props.defaultValue != null) { - node.defaultValue = defaultValue; + if (defaultValue != null) { + node.defaultValue = defaultValueStr; } } else { // Otherwise, the value attribute is synchronized to the property, @@ -244,12 +239,13 @@ export function initInput( node.name = ''; } - const defaultChecked = - props.checked != null ? props.checked : props.defaultChecked; + const checkedOrDefault = checked != null ? checked : defaultChecked; + // TODO: This 'function' or 'symbol' check isn't replicated in other places + // so this semantic is inconsistent. const initialChecked = - typeof defaultChecked !== 'function' && - typeof defaultChecked !== 'symbol' && - !!defaultChecked; + typeof checkedOrDefault !== 'function' && + typeof checkedOrDefault !== 'symbol' && + !!checkedOrDefault; // The checked property never gets assigned. It must be manually set. // We don't want to do this when hydrating so that existing user input isn't @@ -264,9 +260,9 @@ export function initInput( // Only assign the checked attribute if it is defined. This saves // a DOM write when controlling the checked attribute isn't needed // (text inputs, submit/reset) - if (props.defaultChecked != null) { + if (defaultChecked != null) { node.defaultChecked = !node.defaultChecked; - node.defaultChecked = !!props.defaultChecked; + node.defaultChecked = !!defaultChecked; } } else { // When syncing the checked attribute, both the checked property and @@ -285,12 +281,15 @@ export function initInput( } export function restoreControlledInputState(element: Element, props: Object) { - const node: HTMLInputElement = (element: any); - updateInput(node, props); - updateNamedCousins(node, props); -} - -function updateNamedCousins(rootNode: HTMLInputElement, props: any) { + const rootNode: HTMLInputElement = (element: any); + updateInput( + rootNode, + props.value, + props.defaultValue, + props.checked, + props.defaultChecked, + props.type, + ); const name = props.name; if (props.type === 'radio' && name != null) { let queryRoot: Element = rootNode; @@ -322,7 +321,7 @@ function updateNamedCousins(rootNode: HTMLInputElement, props: any) { // and the same name are rendered into the same form (same as #1939). // That's probably okay; we don't support it just as we don't support // mixing React radio buttons with non-React ones. - const otherProps = getFiberCurrentPropsFromNode(otherNode); + const otherProps: any = getFiberCurrentPropsFromNode(otherNode); if (!otherProps) { throw new Error( @@ -338,7 +337,14 @@ function updateNamedCousins(rootNode: HTMLInputElement, props: any) { // If this is a controlled radio button group, forcing the input that // was previously checked to update will cause it to be come re-checked // as appropriate. - updateInput(otherNode, otherProps); + updateInput( + otherNode, + otherProps.value, + otherProps.defaultValue, + otherProps.checked, + otherProps.defaultChecked, + otherProps.type, + ); } } } From 298fe8d4f98f44473e9e397c625ba764f15ef38d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 10 Apr 2023 22:57:59 -0400 Subject: [PATCH 2/3] Use already extracted values instead of reading off props for textarea --- .../src/client/ReactDOMComponent.js | 47 ++++++++++++++----- .../src/client/ReactDOMTextarea.js | 26 ++++++---- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index b9d2a1898399a..e6276b01683e2 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -954,6 +954,9 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); + let value = null; + let defaultValue = null; + let children = null; for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; @@ -964,11 +967,17 @@ export function setInitialProperties( } switch (propKey) { case 'value': { - // This is handled by updateWrapper below. + value = propValue; + // This is handled by initTextarea below. + break; + } + case 'defaultValue': { + defaultValue = propValue; break; } case 'children': { - // TODO: Handled by initWrapperState above. + children = propValue; + // Handled by initTextarea above. break; } case 'dangerouslySetInnerHTML': { @@ -980,7 +989,6 @@ export function setInitialProperties( } break; } - // defaultValue is ignored by setProp default: { setProp(domElement, tag, propKey, propValue, props); } @@ -990,7 +998,7 @@ export function setInitialProperties( // up necessary since we never stop tracking anymore. track((domElement: any)); validateTextareaProps(domElement, props); - initTextarea(domElement, props); + initTextarea(domElement, value, defaultValue, children); return; } case 'option': { @@ -1504,6 +1512,8 @@ export function updateProperties( return; } case 'textarea': { + let value = null; + let defaultValue = null; for (const propKey in lastProps) { const lastProp = lastProps[propKey]; if ( @@ -1513,7 +1523,7 @@ export function updateProperties( ) { switch (propKey) { case 'value': { - // This is handled by updateWrapper below. + // This is handled by updateTextarea below. break; } case 'children': { @@ -1532,12 +1542,16 @@ export function updateProperties( const lastProp = lastProps[propKey]; if ( nextProps.hasOwnProperty(propKey) && - nextProp !== lastProp && (nextProp != null || lastProp != null) ) { switch (propKey) { case 'value': { - // This is handled by updateWrapper below. + value = nextProp; + // This is handled by updateTextarea below. + break; + } + case 'defaultValue': { + defaultValue = nextProp; break; } case 'children': { @@ -1553,14 +1567,21 @@ export function updateProperties( } break; } - // defaultValue is ignored by setProp default: { - setProp(domElement, tag, propKey, nextProp, nextProps, lastProp); + if (nextProp !== lastProp) + setProp( + domElement, + tag, + propKey, + nextProp, + nextProps, + lastProp, + ); } } } } - updateTextarea(domElement, nextProps); + updateTextarea(domElement, value, defaultValue); return; } case 'option': { @@ -1905,6 +1926,8 @@ export function updatePropertiesWithDiff( return; } case 'textarea': { + const value = nextProps.value; + const defaultValue = nextProps.defaultValue; for (let i = 0; i < updatePayload.length; i += 2) { const propKey = updatePayload[i]; const propValue = updatePayload[i + 1]; @@ -1932,7 +1955,7 @@ export function updatePropertiesWithDiff( } } } - updateTextarea(domElement, nextProps); + updateTextarea(domElement, value, defaultValue); return; } case 'option': { @@ -2941,7 +2964,7 @@ export function diffHydratedProperties( // up necessary since we never stop tracking anymore. track((domElement: any)); validateTextareaProps(domElement, props); - initTextarea(domElement, props); + initTextarea(domElement, props.value, props.defaultValue, props.children); break; } diff --git a/packages/react-dom-bindings/src/client/ReactDOMTextarea.js b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js index c9862c9b00bef..d4434b26994fd 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMTextarea.js +++ b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js @@ -58,41 +58,47 @@ export function validateTextareaProps(element: Element, props: Object) { } } -export function updateTextarea(element: Element, props: Object) { +export function updateTextarea( + element: Element, + value: ?string, + defaultValue: ?string, +) { const node: HTMLTextAreaElement = (element: any); - const value = getToStringValue(props.value); if (value != null) { // Cast `value` to a string to ensure the value is set correctly. While // browsers typically do this as necessary, jsdom doesn't. - const newValue = toString(value); + const newValue = toString(getToStringValue(value)); // To avoid side effects (such as losing text selection), only set value if changed if (newValue !== node.value) { node.value = newValue; } // TOOO: This should respect disableInputAttributeSyncing flag. - if (props.defaultValue == null) { + if (defaultValue == null) { if (node.defaultValue !== newValue) { node.defaultValue = newValue; } return; } } - const defaultValue = getToStringValue(props.defaultValue); if (defaultValue != null) { - node.defaultValue = toString(defaultValue); + node.defaultValue = toString(getToStringValue(defaultValue)); } else { node.defaultValue = ''; } } -export function initTextarea(element: Element, props: Object) { +export function initTextarea( + element: Element, + value: ?string, + defaultValue: ?string, + children: ?string, +) { const node: HTMLTextAreaElement = (element: any); - let initialValue = props.value; + let initialValue = value; // Only bother fetching default value if we're going to use it if (initialValue == null) { - let {children, defaultValue} = props; if (children != null) { if (!disableTextareaChildren) { if (defaultValue != null) { @@ -141,5 +147,5 @@ export function restoreControlledTextareaState( props: Object, ) { // DOM component is still mounted; update - updateTextarea(element, props); + updateTextarea(element, props.value, props.defaultValue); } From 79e4988db50073fb1b90d3f3ab6382b1e1713b90 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 10 Apr 2023 23:16:31 -0400 Subject: [PATCH 3/3] Use already extracted values instead of reading off props for select --- .../src/client/ReactDOMComponent.js | 70 +++++++++++++++---- .../src/client/ReactDOMSelect.js | 34 +++++---- 2 files changed, 74 insertions(+), 30 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index e6276b01683e2..bcbc8854166aa 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -924,6 +924,9 @@ export function setInitialProperties( // We listen to this event in case to ensure emulated bubble // listeners still fire for the invalid event. listenToNonDelegatedEvent('invalid', domElement); + let value = null; + let defaultValue = null; + let multiple = null; for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; @@ -934,17 +937,28 @@ export function setInitialProperties( } switch (propKey) { case 'value': { - // This is handled by updateWrapper below. + value = propValue; + // This is handled by initSelect below. break; } - // defaultValue are ignored by setProp + case 'defaultValue': { + defaultValue = propValue; + // This is handled by initSelect below. + break; + } + case 'multiple': { + multiple = propValue; + // TODO: We don't actually have to fall through here because we set it + // in initSelect anyway. We can remove the special case in setProp. + } + // Fallthrough default: { setProp(domElement, tag, propKey, propValue, props, null); } } } validateSelectProps(domElement, props); - initSelect(domElement, props); + initSelect(domElement, value, defaultValue, multiple); return; } case 'textarea': { @@ -1467,21 +1481,27 @@ export function updateProperties( return; } case 'select': { + let value = null; + let defaultValue = null; + let multiple = null; + let wasMultiple = null; for (const propKey in lastProps) { const lastProp = lastProps[propKey]; - if ( - lastProps.hasOwnProperty(propKey) && - lastProp != null && - !nextProps.hasOwnProperty(propKey) - ) { + if (lastProps.hasOwnProperty(propKey) && lastProp != null) { switch (propKey) { case 'value': { // This is handled by updateWrapper below. break; } // defaultValue are ignored by setProp + case 'multiple': { + wasMultiple = lastProp; + // TODO: Move special case in here from setProp. + } + // Fallthrough default: { - setProp(domElement, tag, propKey, null, nextProps, lastProp); + if (!nextProps.hasOwnProperty(propKey)) + setProp(domElement, tag, propKey, null, nextProps, lastProp); } } } @@ -1491,24 +1511,40 @@ export function updateProperties( const lastProp = lastProps[propKey]; if ( nextProps.hasOwnProperty(propKey) && - nextProp !== lastProp && (nextProp != null || lastProp != null) ) { switch (propKey) { case 'value': { - // This is handled by updateWrapper below. + value = nextProp; + // This is handled by updateSelect below. break; } - // defaultValue are ignored by setProp + case 'defaultValue': { + defaultValue = nextProp; + break; + } + case 'multiple': { + multiple = nextProp; + // TODO: Just move the special case in here from setProp. + } + // Fallthrough default: { - setProp(domElement, tag, propKey, nextProp, nextProps, lastProp); + if (nextProp !== lastProp) + setProp( + domElement, + tag, + propKey, + nextProp, + nextProps, + lastProp, + ); } } } } // value update needs to occur after