From 0fa843549b380659b74a0fae32fbd50549c66b88 Mon Sep 17 00:00:00 2001
From: rmorshea <ryan.morshead@gmail.com>
Date: Wed, 2 Mar 2022 00:17:48 -0800
Subject: [PATCH 1/2] actually fix #684

Because we handle events asynchronously, we must leave the value
uncontrolled in order to allow all changed committed by the user to be
recorded in the order they occur. If we don't, the user may commit
multiple changes before we render resulting in prior changes being
overwritten by subsequent ones. Instead of controlling the value, we set
it in an effect. This feels a bit hacky, but I can't think of a better
way to work around this problem.
---
 docs/source/_custom_js/package-lock.json      |  2 +-
 .../idom-client-react/src/components.js       | 41 +++++++++++++++++++
 .../idom-client-react/src/element-utils.js    |  2 -
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/docs/source/_custom_js/package-lock.json b/docs/source/_custom_js/package-lock.json
index 62c2210aa..ceed13e02 100644
--- a/docs/source/_custom_js/package-lock.json
+++ b/docs/source/_custom_js/package-lock.json
@@ -19,7 +19,7 @@
       }
     },
     "../../../src/client/packages/idom-client-react": {
-      "version": "0.37.0",
+      "version": "0.37.1-a1",
       "integrity": "sha512-pIK5eNwFSHKXg7ClpASWFVKyZDYxz59MSFpVaX/OqJFkrJaAxBuhKGXNTMXmuyWOL5Iyvb/ErwwDRxQRzMNkfQ==",
       "license": "MIT",
       "dependencies": {
diff --git a/src/client/packages/idom-client-react/src/components.js b/src/client/packages/idom-client-react/src/components.js
index c44eed085..65e8c8e6c 100644
--- a/src/client/packages/idom-client-react/src/components.js
+++ b/src/client/packages/idom-client-react/src/components.js
@@ -38,6 +38,8 @@ export function Element({ model }) {
     }
   } else if (model.tagName == "script") {
     return html`<${ScriptElement} model=${model} />`;
+  } else if (["input", "select", "textarea"].includes(model.tagName)) {
+    return html`<${UserInputElement} model=${model} />`;
   } else if (model.importSource) {
     return html`<${ImportedElement} model=${model} />`;
   } else {
@@ -68,6 +70,45 @@ function StandardElement({ model }) {
   );
 }
 
+// Element with a value attribute controlled by user input
+function UserInputElement({ model }) {
+  const ref = React.useRef();
+  const layoutContext = React.useContext(LayoutContext);
+
+  const props = createElementAttributes(model, layoutContext.sendEvent);
+
+  // Because we handle events asynchronously, we must leave the value uncontrolled in
+  // order to allow all changes committed by the user to be recorded in the order they
+  // occur. If we don't the user may commit multiple changes before we render next
+  // causing the content of prior changes to be overwritten by subsequent changes.
+  const value = props.value;
+  delete props.value;
+
+  // Instead of controlling the value, we set it in an effect.
+  React.useEffect(() => {
+    if (value !== undefined) {
+      ref.current.value = value;
+    }
+  }, [ref.current, value]);
+
+  // Use createElement here to avoid warning about variable numbers of children not
+  // having keys. Warning about this must now be the responsibility of the server
+  // providing the models instead of the client rendering them.
+  return React.createElement(
+    model.tagName,
+    {
+      ...props,
+      ref: (target) => {
+        ref.current = target;
+      },
+    },
+    ...createElementChildren(
+      model,
+      (model) => html`<${Element} key=${model.key} model=${model} />`
+    )
+  );
+}
+
 function ScriptElement({ model }) {
   const ref = React.useRef();
   React.useEffect(() => {
diff --git a/src/client/packages/idom-client-react/src/element-utils.js b/src/client/packages/idom-client-react/src/element-utils.js
index e9a39458c..cbf13470d 100644
--- a/src/client/packages/idom-client-react/src/element-utils.js
+++ b/src/client/packages/idom-client-react/src/element-utils.js
@@ -39,8 +39,6 @@ function createEventHandler(eventName, sendEvent, eventSpec) {
       if (typeof value === "object" && value.nativeEvent) {
         if (eventSpec["preventDefault"]) {
           value.preventDefault();
-        } else if (eventName === "onChange") {
-          value.nativeEvent.target.value = value.target.value;
         }
         if (eventSpec["stopPropagation"]) {
           value.stopPropagation();

From 9f64ecd17f7d09632903462fd92c04ccf4b3d92d Mon Sep 17 00:00:00 2001
From: rmorshea <ryan.morshead@gmail.com>
Date: Wed, 2 Mar 2022 00:52:27 -0800
Subject: [PATCH 2/2] update changelog

---
 docs/source/about/changelog.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst
index 9757d5bcc..aae86ad8d 100644
--- a/docs/source/about/changelog.rst
+++ b/docs/source/about/changelog.rst
@@ -16,7 +16,9 @@ scheme for the project adheres to `Semantic Versioning <https://semver.org/>`__.
 Unreleased
 ----------
 
-Nothing yet...
+Fixed:
+
+- Revert :pull:`694` and by making ``value`` uncontrolled client-side - :issue:`684`
 
 
 0.37.1-a1
@@ -24,7 +26,7 @@ Nothing yet...
 
 Fixed:
 
-- ``onChange`` event for inputs missing key strokes :issue:`684`
+- ``onChange`` event for inputs missing key strokes - :issue:`684`
 
 
 0.37.0