From 2fa632381839c8732dad9107b90911163b7f2b7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Sun, 23 Apr 2023 17:44:28 -0400
Subject: [PATCH] Restore server controlled form fields to whatever they should
 be (#26708)

Fizz can emit whatever it wants for the SSR version of these fields when
it's a function action so they might not align with what is in the
previous props. Therefore we need to force them to update if we're
updating to a non-function where they might be relevant again.
---
 .../src/client/ReactDOMComponent.js           | 145 ++++++++++++---
 .../src/client/ReactDOMInput.js               |   1 -
 .../src/__tests__/ReactDOMFizzForm-test.js    | 165 ++++++++++++++++++
 3 files changed, 283 insertions(+), 28 deletions(-)

diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
index 0a198d718b172..8d2402e26ba9b 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -483,6 +483,68 @@ function setProp(
     case 'action':
     case 'formAction': {
       // TODO: Consider moving these special cases to the form, input and button tags.
+      if (enableFormActions) {
+        if (typeof value === 'function') {
+          // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
+          // because we'll preventDefault, but it can happen if a form is manually submitted or
+          // if someone calls stopPropagation before React gets the event.
+          // If CSP is used to block javascript: URLs that's fine too. It just won't show this
+          // error message but the URL will be logged.
+          domElement.setAttribute(
+            key,
+            // eslint-disable-next-line no-script-url
+            "javascript:throw new Error('" +
+              'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
+              "consider using form.requestSubmit() instead. If you're trying to use " +
+              'event.stopPropagation() in a submit event handler, consider also calling ' +
+              'event.preventDefault().' +
+              "')",
+          );
+          break;
+        } else if (typeof prevValue === 'function') {
+          // When we're switching off a Server Action that was originally hydrated.
+          // The server control these fields during SSR that are now trailing.
+          // The regular diffing doesn't apply since we compare against the previous props.
+          // Instead, we need to force them to be set to whatever they should be now.
+          // This would be a lot cleaner if we did this whole fork in the per-tag approach.
+          if (key === 'formAction') {
+            if (tag !== 'input') {
+              // Setting the name here isn't completely safe for inputs if this is switching
+              // to become a radio button. In that case we let the tag based override take
+              // control.
+              setProp(domElement, tag, 'name', props.name, props, null);
+            }
+            setProp(
+              domElement,
+              tag,
+              'formEncType',
+              props.formEncType,
+              props,
+              null,
+            );
+            setProp(
+              domElement,
+              tag,
+              'formMethod',
+              props.formMethod,
+              props,
+              null,
+            );
+            setProp(
+              domElement,
+              tag,
+              'formTarget',
+              props.formTarget,
+              props,
+              null,
+            );
+          } else {
+            setProp(domElement, tag, 'encType', props.encType, props, null);
+            setProp(domElement, tag, 'method', props.method, props, null);
+            setProp(domElement, tag, 'target', props.target, props, null);
+          }
+        }
+      }
       if (
         value == null ||
         (!enableFormActions && typeof value === 'function') ||
@@ -495,24 +557,6 @@ function setProp(
       if (__DEV__) {
         validateFormActionInDevelopment(tag, key, value, props);
       }
-      if (enableFormActions && typeof value === 'function') {
-        // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
-        // because we'll preventDefault, but it can happen if a form is manually submitted or
-        // if someone calls stopPropagation before React gets the event.
-        // If CSP is used to block javascript: URLs that's fine too. It just won't show this
-        // error message but the URL will be logged.
-        domElement.setAttribute(
-          key,
-          // eslint-disable-next-line no-script-url
-          "javascript:throw new Error('" +
-            'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
-            "consider using form.requestSubmit() instead. If you're trying to use " +
-            'event.stopPropagation() in a submit event handler, consider also calling ' +
-            'event.preventDefault().' +
-            "')",
-        );
-        break;
-      }
       // `setAttribute` with objects becomes only `[object]` in IE8/9,
       // ('' + value) makes it output the correct toString()-value.
       if (__DEV__) {
@@ -1138,7 +1182,7 @@ export function setInitialProperties(
             break;
           }
           default: {
-            setProp(domElement, tag, propKey, propValue, props);
+            setProp(domElement, tag, propKey, propValue, props, null);
           }
         }
       }
@@ -1169,7 +1213,7 @@ export function setInitialProperties(
             break;
           }
           default: {
-            setProp(domElement, tag, propKey, propValue, props);
+            setProp(domElement, tag, propKey, propValue, props, null);
           }
         }
       }
@@ -1935,7 +1979,14 @@ export function updatePropertiesWithDiff(
             break;
           }
           default: {
-            setProp(domElement, tag, propKey, propValue, nextProps, null);
+            setProp(
+              domElement,
+              tag,
+              propKey,
+              propValue,
+              nextProps,
+              lastProps[propKey],
+            );
           }
         }
       }
@@ -2010,7 +2061,14 @@ export function updatePropertiesWithDiff(
           }
           // defaultValue are ignored by setProp
           default: {
-            setProp(domElement, tag, propKey, propValue, nextProps, null);
+            setProp(
+              domElement,
+              tag,
+              propKey,
+              propValue,
+              nextProps,
+              lastProps[propKey],
+            );
           }
         }
       }
@@ -2045,7 +2103,14 @@ export function updatePropertiesWithDiff(
           }
           // defaultValue is ignored by setProp
           default: {
-            setProp(domElement, tag, propKey, propValue, nextProps, null);
+            setProp(
+              domElement,
+              tag,
+              propKey,
+              propValue,
+              nextProps,
+              lastProps[propKey],
+            );
           }
         }
       }
@@ -2066,7 +2131,14 @@ export function updatePropertiesWithDiff(
             break;
           }
           default: {
-            setProp(domElement, tag, propKey, propValue, nextProps, null);
+            setProp(
+              domElement,
+              tag,
+              propKey,
+              propValue,
+              nextProps,
+              lastProps[propKey],
+            );
           }
         }
       }
@@ -2105,7 +2177,14 @@ export function updatePropertiesWithDiff(
           }
           // defaultChecked and defaultValue are ignored by setProp
           default: {
-            setProp(domElement, tag, propKey, propValue, nextProps, null);
+            setProp(
+              domElement,
+              tag,
+              propKey,
+              propValue,
+              nextProps,
+              lastProps[propKey],
+            );
           }
         }
       }
@@ -2122,7 +2201,7 @@ export function updatePropertiesWithDiff(
             propKey,
             propValue,
             nextProps,
-            null,
+            lastProps[propKey],
           );
         }
         return;
@@ -2134,7 +2213,7 @@ export function updatePropertiesWithDiff(
   for (let i = 0; i < updatePayload.length; i += 2) {
     const propKey = updatePayload[i];
     const propValue = updatePayload[i + 1];
-    setProp(domElement, tag, propKey, propValue, nextProps, null);
+    setProp(domElement, tag, propKey, propValue, nextProps, lastProps[propKey]);
   }
 }
 
@@ -2709,6 +2788,18 @@ function diffHydratedGenericElement(
           const hasFormActionURL = serverValue === EXPECTED_FORM_ACTION_URL;
           if (typeof value === 'function') {
             extraAttributes.delete(propKey.toLowerCase());
+            // The server can set these extra properties to implement actions.
+            // So we remove them from the extra attributes warnings.
+            if (propKey === 'formAction') {
+              extraAttributes.delete('name');
+              extraAttributes.delete('formenctype');
+              extraAttributes.delete('formmethod');
+              extraAttributes.delete('formtarget');
+            } else {
+              extraAttributes.delete('enctype');
+              extraAttributes.delete('method');
+              extraAttributes.delete('target');
+            }
             if (hasFormActionURL) {
               // Expected
               continue;
diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js
index 2096238d762f6..30f06dacd18e1 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMInput.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js
@@ -131,7 +131,6 @@ export function updateInput(
     // Submit/reset inputs need the attribute removed completely to avoid
     // blank-text buttons.
     node.removeAttribute('value');
-    return;
   }
 
   if (disableInputAttributeSyncing) {
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
index a73c29f0abe2c..efb3a1de960e8 100644
--- a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
@@ -195,4 +195,169 @@ describe('ReactDOMFizzForm', () => {
       'Prop `action` did not match. Server: "action" Client: "function action(formData) {}"',
     );
   });
+
+  // @gate enableFormActions || !__DEV__
+  it('should reset form fields after you update away from hydrated function', async () => {
+    const formRef = React.createRef();
+    const inputRef = React.createRef();
+    const buttonRef = React.createRef();
+    function action(formData) {}
+    function App({isUpdate}) {
+      return (
+        <form
+          action={isUpdate ? 'action' : action}
+          ref={formRef}
+          method={isUpdate ? 'POST' : null}>
+          <input
+            type="submit"
+            formAction={isUpdate ? 'action' : action}
+            ref={inputRef}
+            formTarget={isUpdate ? 'elsewhere' : null}
+          />
+          <button
+            formAction={isUpdate ? 'action' : action}
+            ref={buttonRef}
+            formEncType={isUpdate ? 'multipart/form-data' : null}
+          />
+        </form>
+      );
+    }
+
+    const stream = await ReactDOMServer.renderToReadableStream(<App />);
+    await readIntoContainer(stream);
+    let root;
+    await act(async () => {
+      root = ReactDOMClient.hydrateRoot(container, <App />);
+    });
+    await act(async () => {
+      root.render(<App isUpdate={true} />);
+    });
+    expect(formRef.current.getAttribute('action')).toBe('action');
+    expect(formRef.current.hasAttribute('encType')).toBe(false);
+    expect(formRef.current.getAttribute('method')).toBe('POST');
+    expect(formRef.current.hasAttribute('target')).toBe(false);
+
+    expect(inputRef.current.getAttribute('formAction')).toBe('action');
+    expect(inputRef.current.hasAttribute('name')).toBe(false);
+    expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
+    expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(inputRef.current.getAttribute('formTarget')).toBe('elsewhere');
+
+    expect(buttonRef.current.getAttribute('formAction')).toBe('action');
+    expect(buttonRef.current.hasAttribute('name')).toBe(false);
+    expect(buttonRef.current.getAttribute('formEncType')).toBe(
+      'multipart/form-data',
+    );
+    expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
+  });
+
+  // @gate enableFormActions || !__DEV__
+  it('should reset form fields after you remove a hydrated function', async () => {
+    const formRef = React.createRef();
+    const inputRef = React.createRef();
+    const buttonRef = React.createRef();
+    function action(formData) {}
+    function App({isUpdate}) {
+      return (
+        <form action={isUpdate ? undefined : action} ref={formRef}>
+          <input
+            type="submit"
+            formAction={isUpdate ? undefined : action}
+            ref={inputRef}
+          />
+          <button formAction={isUpdate ? undefined : action} ref={buttonRef} />
+        </form>
+      );
+    }
+
+    const stream = await ReactDOMServer.renderToReadableStream(<App />);
+    await readIntoContainer(stream);
+    let root;
+    await act(async () => {
+      root = ReactDOMClient.hydrateRoot(container, <App />);
+    });
+    await act(async () => {
+      root.render(<App isUpdate={true} />);
+    });
+    expect(formRef.current.hasAttribute('action')).toBe(false);
+    expect(formRef.current.hasAttribute('encType')).toBe(false);
+    expect(formRef.current.hasAttribute('method')).toBe(false);
+    expect(formRef.current.hasAttribute('target')).toBe(false);
+
+    expect(inputRef.current.hasAttribute('formAction')).toBe(false);
+    expect(inputRef.current.hasAttribute('name')).toBe(false);
+    expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
+    expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(inputRef.current.hasAttribute('formTarget')).toBe(false);
+
+    expect(buttonRef.current.hasAttribute('formAction')).toBe(false);
+    expect(buttonRef.current.hasAttribute('name')).toBe(false);
+    expect(buttonRef.current.hasAttribute('formEncType')).toBe(false);
+    expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
+  });
+
+  // @gate enableFormActions || !__DEV__
+  it('should restore the form fields even if they were incorrectly set', async () => {
+    const formRef = React.createRef();
+    const inputRef = React.createRef();
+    const buttonRef = React.createRef();
+    function action(formData) {}
+    function App({isUpdate}) {
+      return (
+        <form
+          action={isUpdate ? 'action' : action}
+          ref={formRef}
+          method="DELETE">
+          <input
+            type="submit"
+            formAction={isUpdate ? 'action' : action}
+            ref={inputRef}
+            formTarget="elsewhere"
+          />
+          <button
+            formAction={isUpdate ? 'action' : action}
+            ref={buttonRef}
+            formEncType="text/plain"
+          />
+        </form>
+      );
+    }
+
+    // Specifying the extra form fields are a DEV error, but we expect it
+    // to eventually still be patched up after an update.
+    await expect(async () => {
+      const stream = await ReactDOMServer.renderToReadableStream(<App />);
+      await readIntoContainer(stream);
+    }).toErrorDev([
+      'Cannot specify a encType or method for a form that specifies a function as the action.',
+      'Cannot specify a formTarget for a button that specifies a function as a formAction.',
+    ]);
+    let root;
+    await expect(async () => {
+      await act(async () => {
+        root = ReactDOMClient.hydrateRoot(container, <App />);
+      });
+    }).toErrorDev(['Prop `formTarget` did not match.']);
+    await act(async () => {
+      root.render(<App isUpdate={true} />);
+    });
+    expect(formRef.current.getAttribute('action')).toBe('action');
+    expect(formRef.current.hasAttribute('encType')).toBe(false);
+    expect(formRef.current.getAttribute('method')).toBe('DELETE');
+    expect(formRef.current.hasAttribute('target')).toBe(false);
+
+    expect(inputRef.current.getAttribute('formAction')).toBe('action');
+    expect(inputRef.current.hasAttribute('name')).toBe(false);
+    expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
+    expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(inputRef.current.getAttribute('formTarget')).toBe('elsewhere');
+
+    expect(buttonRef.current.getAttribute('formAction')).toBe('action');
+    expect(buttonRef.current.hasAttribute('name')).toBe(false);
+    expect(buttonRef.current.getAttribute('formEncType')).toBe('text/plain');
+    expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
+    expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
+  });
 });