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.
2026-09-24 17:02:27 +00:00 · 2023-04-23 17:44:28 -04:00
parent a6a29acf7a
commit 731237bea7
3 changed files with 283 additions and 28 deletions
@@ -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;
@@ -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) {
@@ -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);
+  });
 });