Allow an action provide a custom set of props to use for progressive enhancement

Author: sebmarkbage

packages/react-dom-bindings/src/client/ReactDOMComponent.js

@@ -2791,7 +2791,6 @@ function diffHydratedGenericElement(
       case 'formAction':
         if (enableFormActions) {
           const serverValue = domElement.getAttribute(propKey);
-          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.
@@ -2806,13 +2805,14 @@ function diffHydratedGenericElement(
               extraAttributes.delete('method');
               extraAttributes.delete('target');
             }
-            if (hasFormActionURL) {
-              // Expected
-              continue;
-            }
-            warnForPropDifference(propKey, serverValue, value);
+            // Ideally we should be able to warn if the server value was not a function
+            // however since the function can return any of these attributes any way it
+            // wants as a custom progressive enhancement, there's nothing to compare to.
+            // We can check if the function has the $FORM_ACTION property on the client
+            // and if it's not, warn, but that's an unnecessary constraint that they
+            // have to have the extra extension that doesn't do anything on the client.
             continue;
-          } else if (hasFormActionURL) {
+          } else if (serverValue === EXPECTED_FORM_ACTION_URL) {
             extraAttributes.delete(propKey.toLowerCase());
             warnForPropDifference(propKey, 'function', value);
             continue;

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

@@ -1445,7 +1445,7 @@ export function shouldDeleteUnhydratedTailInstances(
   return (
     (enableHostSingletons ||
       (parentType !== 'head' && parentType !== 'body')) &&
-    (!enableFormActions || parentType !== 'form')
+    (!enableFormActions || (parentType !== 'form' && parentType !== 'button'))
   );
 }
 

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

@@ -632,6 +632,22 @@ function pushStringAttribute(
   }
 }
 
+type CustomFormAction = {
+  name?: string,
+  action?: string,
+  encType?: string,
+  method?: string,
+  target?: string,
+  data?: FormData,
+};
+
+function makeFormFieldPrefix(responseState: ResponseState): string {
+  // I'm just reusing this counter. It's not really the same namespace as "name".
+  // It could just be its own counter.
+  const id = responseState.nextSuspenseID++;
+  return responseState.idPrefix + '$ACTION:' + id + ':';
+}
+
 // Since this will likely be repeated a lot in the HTML, we use a more concise message
 // than on the client and hopefully it's googleable.
 const actionJavaScriptURL = stringToPrecomputedChunk(
@@ -641,6 +657,36 @@ const actionJavaScriptURL = stringToPrecomputedChunk(
   ),
 );
 
+const startHiddenInputChunk = stringToPrecomputedChunk('<input type="hidden"');
+
+function pushAdditionalFormField(
+  this: Array<Chunk | PrecomputedChunk>,
+  value: string | File,
+  key: string,
+): void {
+  const target: Array<Chunk | PrecomputedChunk> = this;
+  target.push(startHiddenInputChunk);
+  if (typeof value !== 'string') {
+    throw new Error(
+      'File/Blob fields are not yet supported in progressive forms. ' +
+        'It probably means you are closing over binary data or FormData in a Server Action.',
+    );
+  }
+  pushStringAttribute(target, 'name', key);
+  pushStringAttribute(target, 'value', value);
+  target.push(endOfStartTagSelfClosing);
+}
+
+function pushAdditionalFormFields(
+  target: Array<Chunk | PrecomputedChunk>,
+  formData: null | FormData,
+) {
+  if (formData !== null) {
+    // $FlowFixMe[prop-missing]: FormData has forEach.
+    formData.forEach(pushAdditionalFormField, target);
+  }
+}
+
 function pushFormActionAttribute(
   target: Array<Chunk | PrecomputedChunk>,
   responseState: ResponseState,
@@ -649,7 +695,8 @@ function pushFormActionAttribute(
   formMethod: any,
   formTarget: any,
   name: any,
-): void {
+): null | FormData {
+  let formData = null;
   if (enableFormActions && typeof formAction === 'function') {
     // Function form actions cannot control the form properties
     if (__DEV__) {
@@ -678,37 +725,55 @@ function pushFormActionAttribute(
         );
       }
     }
-    // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
-    // because we'll preventDefault in the Fizz runtime, 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.
-    target.push(
-      attributeSeparator,
-      stringToChunk('formAction'),
-      attributeAssign,
-      actionJavaScriptURL,
-      attributeEnd,
-    );
-    injectFormReplayingRuntime(responseState);
-  } else {
-    // Plain form actions support all the properties, so we have to emit them.
-    if (name !== null) {
-      pushAttribute(target, 'name', name);
-    }
-    if (formAction !== null) {
-      pushAttribute(target, 'formAction', formAction);
-    }
-    if (formEncType !== null) {
-      pushAttribute(target, 'formEncType', formEncType);
-    }
-    if (formMethod !== null) {
-      pushAttribute(target, 'formMethod', formMethod);
-    }
-    if (formTarget !== null) {
-      pushAttribute(target, 'formTarget', formTarget);
+    const customAction: CustomFormAction = formAction.$$FORM_ACTION;
+    if (typeof customAction === 'function') {
+      // This action has a custom progressive enhancement form that can submit the form
+      // back to the server if it's invoked before hydration. Such as a Server Action.
+      const prefix = makeFormFieldPrefix(responseState);
+      const customFields = customAction(prefix);
+      name = customFields.name;
+      formAction = customFields.action || '';
+      formEncType = customFields.encType;
+      formMethod = customFields.method;
+      formTarget = customFields.target;
+      formData = customFields.data;
+    } else {
+      // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
+      // because we'll preventDefault in the Fizz runtime, 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.
+      target.push(
+        attributeSeparator,
+        stringToChunk('formAction'),
+        attributeAssign,
+        actionJavaScriptURL,
+        attributeEnd,
+      );
+      name = null;
+      formAction = null;
+      formEncType = null;
+      formMethod = null;
+      formTarget = null;
+      injectFormReplayingRuntime(responseState);
     }
   }
+  if (name !== null) {
+    pushAttribute(target, 'name', name);
+  }
+  if (formAction !== null) {
+    pushAttribute(target, 'formAction', formAction);
+  }
+  if (formEncType !== null) {
+    pushAttribute(target, 'formEncType', formEncType);
+  }
+  if (formMethod !== null) {
+    pushAttribute(target, 'formMethod', formMethod);
+  }
+  if (formTarget !== null) {
+    pushAttribute(target, 'formTarget', formTarget);
+  }
+  return formData;
 }
 
 function pushAttribute(
@@ -1330,6 +1395,8 @@ function pushStartForm(
     }
   }
 
+  let formData = null;
+  let formActionName = null;
   if (enableFormActions && typeof formAction === 'function') {
     // Function form actions cannot control the form properties
     if (__DEV__) {
@@ -1352,36 +1419,60 @@ function pushStartForm(
         );
       }
     }
-    // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
-    // because we'll preventDefault in the Fizz runtime, 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.
-    target.push(
-      attributeSeparator,
-      stringToChunk('action'),
-      attributeAssign,
-      actionJavaScriptURL,
-      attributeEnd,
-    );
-    injectFormReplayingRuntime(responseState);
-  } else {
-    // Plain form actions support all the properties, so we have to emit them.
-    if (formAction !== null) {
-      pushAttribute(target, 'action', formAction);
-    }
-    if (formEncType !== null) {
-      pushAttribute(target, 'encType', formEncType);
-    }
-    if (formMethod !== null) {
-      pushAttribute(target, 'method', formMethod);
-    }
-    if (formTarget !== null) {
-      pushAttribute(target, 'target', formTarget);
+    const customAction: CustomFormAction = formAction.$$FORM_ACTION;
+    if (typeof customAction === 'function') {
+      // This action has a custom progressive enhancement form that can submit the form
+      // back to the server if it's invoked before hydration. Such as a Server Action.
+      const prefix = makeFormFieldPrefix(responseState);
+      const customFields = customAction(prefix);
+      formAction = customFields.action || '';
+      formEncType = customFields.encType;
+      formMethod = customFields.method;
+      formTarget = customFields.target;
+      formData = customFields.data;
+      formActionName = customFields.name;
+    } else {
+      // Set a javascript URL that doesn't do anything. We don't expect this to be invoked
+      // because we'll preventDefault in the Fizz runtime, 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.
+      target.push(
+        attributeSeparator,
+        stringToChunk('action'),
+        attributeAssign,
+        actionJavaScriptURL,
+        attributeEnd,
+      );
+      formAction = null;
+      formEncType = null;
+      formMethod = null;
+      formTarget = null;
+      injectFormReplayingRuntime(responseState);
     }
   }
+  if (formAction !== null) {
+    pushAttribute(target, 'action', formAction);
+  }
+  if (formEncType !== null) {
+    pushAttribute(target, 'encType', formEncType);
+  }
+  if (formMethod !== null) {
+    pushAttribute(target, 'method', formMethod);
+  }
+  if (formTarget !== null) {
+    pushAttribute(target, 'target', formTarget);
+  }
 
   target.push(endOfStartTag);
+
+  if (formActionName !== null) {
+    target.push(startHiddenInputChunk);
+    pushStringAttribute(target, 'name', formActionName);
+    target.push(endOfStartTagSelfClosing);
+    pushAdditionalFormFields(target, formData);
+  }
+
   pushInnerHTML(target, innerHTML, children);
   if (typeof children === 'string') {
     // Special case children as a string to avoid the unnecessary comment.
@@ -1474,7 +1565,7 @@ function pushInput(
     }
   }
 
-  pushFormActionAttribute(
+  const formData = pushFormActionAttribute(
     target,
     responseState,
     formAction,
@@ -1525,6 +1616,10 @@ function pushInput(
   }
 
   target.push(endOfStartTagSelfClosing);
+
+  // We place any additional hidden form fields after the input.
+  pushAdditionalFormFields(target, formData);
+
   return null;
 }
 
@@ -1592,7 +1687,7 @@ function pushStartButton(
     }
   }
 
-  pushFormActionAttribute(
+  const formData = pushFormActionAttribute(
     target,
     responseState,
     formAction,
@@ -1603,13 +1698,18 @@ function pushStartButton(
   );
 
   target.push(endOfStartTag);
+
+  // We place any additional hidden form fields we need to include inside the button itself.
+  pushAdditionalFormFields(target, formData);
+
   pushInnerHTML(target, innerHTML, children);
   if (typeof children === 'string') {
     // Special case children as a string to avoid the unnecessary comment.
     // TODO: Remove this special case after the general optimization is in place.
     target.push(stringToChunk(encodeHTMLTextNode(children)));
     return null;
   }
+
   return children;
 }