From 1d5a482b05652fd3ee3cbe4a72629a15950daa18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 11 Apr 2023 12:39:00 -0400
Subject: [PATCH] Remove initOption special case (#26595)

This traces back to https://github.com/facebook/react/pull/6449 and then
another before that.

I think that back then we favored the property over the attribute, and
setting the property wouldn't be enough. However, the default path for
these are now using attributes if we don't special case it. So we don't
need it.

The only difference is that we currently have a divergence for
symbol/function behavior between controlled values that use the
getToStringValue helpers which treat them as empty string, where as
everywhere else they're treated as null/missing.

Since this comes with a warning and is a weird error case, it's probably
fine to change.
---
 .../src/client/ReactDOMComponent.js           |  3 +-
 .../src/client/ReactDOMOption.js              |  8 -----
 .../src/__tests__/ReactDOMSelect-test.js      | 29 ++++++++++++-------
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
index e2ba29ceb3aed..91bab1c90f2ef 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -32,7 +32,7 @@ import {
   updateInput,
   restoreControlledInputState,
 } from './ReactDOMInput';
-import {initOption, validateOptionProps} from './ReactDOMOption';
+import {validateOptionProps} from './ReactDOMOption';
 import {
   validateSelectProps,
   initSelect,
@@ -995,7 +995,6 @@ export function setInitialProperties(
           }
         }
       }
-      initOption(domElement, props);
       return;
     }
     case 'dialog': {
diff --git a/packages/react-dom-bindings/src/client/ReactDOMOption.js b/packages/react-dom-bindings/src/client/ReactDOMOption.js
index 7907609ad88fe..4f7d3105a8ef5 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMOption.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMOption.js
@@ -8,7 +8,6 @@
  */
 
 import {Children} from 'react';
-import {getToStringValue, toString} from './ToStringValue';
 
 let didWarnSelectedSetOnOption = false;
 let didWarnInvalidChild = false;
@@ -59,10 +58,3 @@ export function validateOptionProps(element: Element, props: Object) {
     }
   }
 }
-
-export function initOption(element: Element, props: Object) {
-  // value="" should make a value attribute (#6219)
-  if (props.value != null) {
-    element.setAttribute('value', toString(getToStringValue(props.value)));
-  }
-}
diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
index 901d20f8c2d83..9037028b9f037 100644
--- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js
@@ -9,6 +9,13 @@
 
 'use strict';
 
+// Fix JSDOM. setAttribute is supposed to throw on things that can't be implicitly toStringed.
+const setAttribute = Element.prototype.setAttribute;
+Element.prototype.setAttribute = function (name, value) {
+  // eslint-disable-next-line react-internal/safe-string-coercion
+  return setAttribute.call(this, name, '' + value);
+};
+
 describe('ReactDOMSelect', () => {
   let React;
   let ReactDOM;
@@ -849,7 +856,7 @@ describe('ReactDOMSelect', () => {
   });
 
   describe('When given a Symbol value', () => {
-    it('treats initial Symbol value as an empty string', () => {
+    it('treats initial Symbol value as missing', () => {
       let node;
 
       expect(() => {
@@ -862,10 +869,10 @@ describe('ReactDOMSelect', () => {
         );
       }).toErrorDev('Invalid value for prop `value`');
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A Symbol!');
     });
 
-    it('treats updated Symbol value as an empty string', () => {
+    it('treats updated Symbol value as missing', () => {
       let node;
 
       expect(() => {
@@ -888,7 +895,7 @@ describe('ReactDOMSelect', () => {
         </select>,
       );
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A Symbol!');
     });
 
     it('treats initial Symbol defaultValue as an empty string', () => {
@@ -904,7 +911,7 @@ describe('ReactDOMSelect', () => {
         );
       }).toErrorDev('Invalid value for prop `value`');
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A Symbol!');
     });
 
     it('treats updated Symbol defaultValue as an empty string', () => {
@@ -930,12 +937,12 @@ describe('ReactDOMSelect', () => {
         </select>,
       );
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A Symbol!');
     });
   });
 
   describe('When given a function value', () => {
-    it('treats initial function value as an empty string', () => {
+    it('treats initial function value as missing', () => {
       let node;
 
       expect(() => {
@@ -948,7 +955,7 @@ describe('ReactDOMSelect', () => {
         );
       }).toErrorDev('Invalid value for prop `value`');
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A function!');
     });
 
     it('treats initial function defaultValue as an empty string', () => {
@@ -964,7 +971,7 @@ describe('ReactDOMSelect', () => {
         );
       }).toErrorDev('Invalid value for prop `value`');
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A function!');
     });
 
     it('treats updated function value as an empty string', () => {
@@ -990,7 +997,7 @@ describe('ReactDOMSelect', () => {
         </select>,
       );
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A function!');
     });
 
     it('treats updated function defaultValue as an empty string', () => {
@@ -1016,7 +1023,7 @@ describe('ReactDOMSelect', () => {
         </select>,
       );
 
-      expect(node.value).toBe('');
+      expect(node.value).toBe('A function!');
     });
   });