Repository: facebook/react
Pull request: Allow forms to skip hydration of hidden inputs

This allows us to emit extra ephemeral data that will only be used on server rendered forms.

First I refactored the shouldSkip functions to now just do that work inside the canHydrate methods. This makes the Config bindings a little less surface area but it also helps us optimize a bit since we now can look at the code together and find shared paths.

canHydrate returns the instance if it matches, that used to just be there to refine the type but it can also be used to just return a different instance later that we find. If we don't find one, we'll bail out and error regardless so no need to skip past anything.

Changed files:
filename: packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
patch: @@ -89,6 +89,7 @@ import {
   enableHostSingletons,
   enableTrustedTypesIntegration,
   diffInCommitPhase,
+  enableFormActions,
 } from 'shared/ReactFeatureFlags';
 import {
   HostComponent,
@@ -1038,160 +1039,182 @@ export function isHydratableText(text: string): boolean {
   return text !== '';
 }
 
-export function shouldSkipHydratableForInstance(
+export function canHydrateInstance(
   instance: HydratableInstance,
   type: string,
   props: Props,
-): boolean {
-  if (instance.nodeType !== ELEMENT_NODE) {
-    // This is a suspense boundary or Text node.
-    // Suspense Boundaries are never expected to be injected by 3rd parties. If we see one it should be matched
-    // and this is a hydration error.
-    // Text Nodes are also not expected to be injected by 3rd parties. This is less of a guarantee for <body>
-    // but it seems reasonable and conservative to reject this as a hydration error as well
-    return false;
-  } else if (
-    instance.nodeName.toLowerCase() !== type.toLowerCase() ||
-    isMarkedHoistable(instance)
-  ) {
-    // We are either about to
-    return true;
-  } else {
-    // We have an Element with the right type.
+  inRootOrSingleton: boolean,
+): null | Instance {
+  while (instance.nodeType === ELEMENT_NODE) {
     const element: Element = (instance: any);
     const anyProps = (props: any);
-
-    // We are going to try to exclude it if we can definitely identify it as a hoisted Node or if
-    // we can guess that the node is likely hoisted or was inserted by a 3rd party script or browser extension
-    // using high entropy attributes for certain types. This technique will fail for strange insertions like
-    // extension prepending <div> in the <body> but that already breaks before and that is an edge case.
-    switch (type) {
-      // case 'title':
-      //We assume all titles are matchable. You should only have one in the Document, at least in a hoistable scope
-      // and if you are a HostComponent with type title we must either be in an <svg> context or this title must have an `itemProp` prop.
-      case 'meta': {
-        // The only way to opt out of hoisting meta tags is to give it an itemprop attribute. We assume there will be
-        // not 3rd party meta tags that are prepended, accepting the cases where this isn't true because meta tags
-        // are usually only functional for SSR so even in a rare case where we did bind to an injected tag the runtime
-        // implications are minimal
-        if (!element.hasAttribute('itemprop')) {
-          // This is a Hoistable
-          return true;
-        }
-        break;
-      }
-      case 'link': {
-        // Links come in many forms and we do expect 3rd parties to inject them into <head> / <body>. We exclude known resources
-        // and then use high-entroy attributes like href which are almost always used and almost always unique to filter out unlikely
-        // matches.
-        const rel = element.getAttribute('rel');
-        if (rel === 'stylesheet' && element.hasAttribute('data-precedence')) {
-          // This is a stylesheet resource
-          return true;
-        } else if (
-          rel !== anyProps.rel ||
-          element.getAttribute('href') !==
-            (anyProps.href == null ? null : anyProps.href) ||
-          element.getAttribute('crossorigin') !==
-            (anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
-          element.getAttribute('title') !==
-            (anyProps.title == null ? null : anyProps.title)
+    if (element.nodeName.toLowerCase() !== type.toLowerCase()) {
+      if (!inRootOrSingleton || !enableHostSingletons) {
+        // Usually we error for mismatched tags.
+        if (
+          enableFormActions &&
+          element.nodeName === 'INPUT' &&
+          (element: any).type === 'hidden'
         ) {
-          // rel + href should usually be enough to uniquely identify a link however crossOrigin can vary for rel preconnect
-          // and title could vary for rel alternate
-          return true;
+          // If we have extra hidden inputs, we don't mismatch. This allows us to embed
+          // extra form data in the original form.
+        } else {
+          return null;
         }
-        break;
       }
-      case 'style': {
-        // Styles are hard to match correctly. We can exclude known resources but otherwise we accept the fact that a non-hoisted style tags
-        // in <head> or <body> are likely never going to be unmounted given their position in the document and the fact they likely hold global styles
-        if (element.hasAttribute('data-precedence')) {
-          // This is a style resource
-          return true;
-        }
-        break;
+      // In root or singleton parents we skip past mismatched instances.
+    } else if (!inRootOrSingleton || !enableHostSingletons) {
+      // Match
+      if (
+        enableFormActions &&
+        type === 'input' &&
+        (element: any).type === 'hidden' &&
+        anyProps.type !== 'hidden'
+      ) {
+        // Skip past hidden inputs unless that's what we're looking for. This allows us
+        // embed extra form data in the original form.
+      } else {
+        return element;
       }
-      case 'script': {
-        // Scripts are a little tricky, we exclude known resources and then similar to links try to use high-entropy attributes
-        // to reject poor matches. One challenge with scripts are inline scripts. We don't attempt to check text content which could
-        // in theory lead to a hydration error later if a 3rd party injected an inline script before the React rendered nodes.
-        // Falling back to client rendering if this happens should be seemless though so we will try this hueristic and revisit later
-        // if we learn it is problematic
-        const srcAttr = element.getAttribute('src');
-        if (
-          srcAttr &&
-          element.hasAttribute('async') &&
-          !element.hasAttribute('itemprop')
-        ) {
-          // This is an async script resource
-          return true;
-        } else if (
-          srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
-          element.getAttribute('type') !==
-            (anyProps.type == null ? null : anyProps.type) ||
-          element.getAttribute('crossorigin') !==
-            (anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
-        ) {
-          // This script is for a different src
-          return true;
+    } else if (isMarkedHoistable(element)) {
+      // We've already claimed this as a hoistable which isn't hydrated this way so we skip past it.
+    } else {
+      // We have an Element with the right type.
+
+      // We are going to try to exclude it if we can definitely identify it as a hoisted Node or if
+      // we can guess that the node is likely hoisted or was inserted by a 3rd party script or browser extension
+      // using high entropy attributes for certain types. This technique will fail for strange insertions like
+      // extension prepending <div> in the <body> but that already breaks before and that is an edge case.
+      switch (type) {
+        // case 'title':
+        //We assume all titles are matchable. You should only have one in the Document, at least in a hoistable scope
+        // and if you are a HostComponent with type title we must either be in an <svg> context or this title must have an `itemProp` prop.
+        case 'meta': {
+          // The only way to opt out of hoisting meta tags is to give it an itemprop attribute. We assume there will be
+          // not 3rd party meta tags that are prepended, accepting the cases where this isn't true because meta tags
+          // are usually only functional for SSR so even in a rare case where we did bind to an injected tag the runtime
+          // implications are minimal
+          if (!element.hasAttribute('itemprop')) {
+            // This is a Hoistable
+            break;
+          }
+          return element;
+        }
+        case 'link': {
+          // Links come in many forms and we do expect 3rd parties to inject them into <head> / <body>. We exclude known resources
+          // and then use high-entroy attributes like href which are almost always used and almost always unique to filter out unlikely
+          // matches.
+          const rel = element.getAttribute('rel');
+          if (rel === 'stylesheet' && element.hasAttribute('data-precedence')) {
+            // This is a stylesheet resource
+            break;
+          } else if (
+            rel !== anyProps.rel ||
+            element.getAttribute('href') !==
+              (anyProps.href == null ? null : anyProps.href) ||
+            element.getAttribute('crossorigin') !==
+              (anyProps.crossOrigin == null ? null : anyProps.crossOrigin) ||
+            element.getAttribute('title') !==
+              (anyProps.title == null ? null : anyProps.title)
+          ) {
+            // rel + href should usually be enough to uniquely identify a link however crossOrigin can vary for rel preconnect
+            // and title could vary for rel alternate
+            break;
+          }
+          return element;
+        }
+        case 'style': {
+          // Styles are hard to match correctly. We can exclude known resources but otherwise we accept the fact that a non-hoisted style tags
+          // in <head> or <body> are likely never going to be unmounted given their position in the document and the fact they likely hold global styles
+          if (element.hasAttribute('data-precedence')) {
+            // This is a style resource
+            break;
+          }
+          return element;
+        }
+        case 'script': {
+          // Scripts are a little tricky, we exclude known resources and then similar to links try to use high-entropy attributes
+          // to reject poor matches. One challenge with scripts are inline scripts. We don't attempt to check text content which could
+          // in theory lead to a hydration error later if a 3rd party injected an inline script before the React rendered nodes.
+          // Falling back to client rendering if this happens should be seemless though so we will try this hueristic and revisit later
+          // if we learn it is problematic
+          const srcAttr = element.getAttribute('src');
+          if (
+            srcAttr &&
+            element.hasAttribute('async') &&
+            !element.hasAttribute('itemprop')
+          ) {
+            // This is an async script resource
+            break;
+          } else if (
+            srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
+            element.getAttribute('type') !==
+              (anyProps.type == null ? null : anyProps.type) ||
+            element.getAttribute('crossorigin') !==
+              (anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
+          ) {
+            // This script is for a different src
+            break;
+          }
+          return element;
+        }
+        default: {
+          // We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
+          // and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
+          // that should work in the vast majority of cases.
+          return element;
         }
-        break;
       }
     }
-    // We have excluded the most likely cases of mismatch between hoistable tags, 3rd party script inserted tags,
-    // and browser extension inserted tags. While it is possible this is not the right match it is a decent hueristic
-    // that should work in the vast majority of cases.
-    return false;
-  }
-}
-
-export function shouldSkipHydratableForTextInstance(
-  instance: HydratableInstance,
-): boolean {
-  return instance.nodeType === ELEMENT_NODE;
-}
-
-export function shouldSkipHydratableForSuspenseInstance(
-  instance: HydratableInstance,
-): boolean {
-  return instance.nodeType === ELEMENT_NODE;
-}
-
-export function canHydrateInstance(
-  instance: HydratableInstance,
-  type: string,
-  props: Props,
-): null | Instance {
-  if (
-    instance.nodeType !== ELEMENT_NODE ||
-    instance.nodeName.toLowerCase() !== type.toLowerCase()
-  ) {
-    return null;
-  } else {
-    return ((instance: any): Instance);
+    const nextInstance = getNextHydratableSibling(element);
+    if (nextInstance === null) {
+      break;
+    }
+    instance = nextInstance;
   }
+  // This is a suspense boundary or Text node or we got the end.
+  // Suspense Boundaries are never expected to be injected by 3rd parties. If we see one it should be matched
+  // and this is a hydration error.
+  // Text Nodes are also not expected to be injected by 3rd parties. This is less of a guarantee for <body>
+  // but it seems reasonable and conservative to reject this as a hydration error as well
+  return null;
 }
 
 export function canHydrateTextInstance(
   instance: HydratableInstance,
   text: string,
+  inRootOrSingleton: boolean,
 ): null | TextInstance {
+  // Empty strings are not parsed by HTML so there won't be a correct match here.
   if (text === '') return null;
 
-  if (instance.nodeType !== TEXT_NODE) {
-    // Empty strings are not parsed by HTML so there won't be a correct match here.
-    return null;
+  while (instance.nodeType !== TEXT_NODE) {
+    if (!inRootOrSingleton || !enableHostSingletons) {
+      return null;
+    }
+    const nextInstance = getNextHydratableSibling(instance);
+    if (nextInstance === null) {
+      return null;
+    }
+    instance = nextInstance;
   }
   // This has now been refined to a text node.
   return ((instance: any): TextInstance);
 }
 
 export function canHydrateSuspenseInstance(
   instance: HydratableInstance,
+  inRootOrSingleton: boolean,
 ): null | SuspenseInstance {
-  if (instance.nodeType !== COMMENT_NODE) {
-    return null;
+  while (instance.nodeType !== COMMENT_NODE) {
+    if (!inRootOrSingleton || !enableHostSingletons) {
+      return null;
+    }
+    const nextInstance = getNextHydratableSibling(instance);
+    if (nextInstance === null) {
+      return null;
+    }
+    instance = nextInstance;
   }
   // This has now been refined to a suspense node.
   return ((instance: any): SuspenseInstance);
@@ -1416,12 +1439,14 @@ export function commitHydratedSuspenseInstance(
   retryIfBlockedOn(suspenseInstance);
 }
 
-// @TODO remove this function once float lands and hydrated tail nodes
-// are controlled by HostSingleton fibers
 export function shouldDeleteUnhydratedTailInstances(
   parentType: string,
 ): boolean {
-  return parentType !== 'head' && parentType !== 'body';
+  return (
+    (enableHostSingletons ||
+      (parentType !== 'head' && parentType !== 'body')) &&
+    (!enableFormActions || parentType !== 'form')
+  );
 }
 
 export function didNotMatchHydratedContainerTextInstance(

filename: packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js
patch: @@ -695,4 +695,41 @@ describe('ReactDOMServerHydration', () => {
       );
     }
   });
+
+  // @gate enableFormActions
+  it('allows rendering extra hidden inputs in a form', async () => {
+    const element = document.createElement('div');
+    element.innerHTML =
+      '<form>' +
+      '<input type="hidden" /><input type="hidden" name="a" value="A" />' +
+      '<input type="hidden" /><input type="submit" name="b" value="B" />' +
+      '<input type="hidden" /><button name="c" value="C"></button>' +
+      '<input type="hidden" />' +
+      '</form>';
+    const form = element.firstChild;
+    const ref = React.createRef();
+    const a = React.createRef();
+    const b = React.createRef();
+    const c = React.createRef();
+    await act(async () => {
+      ReactDOMClient.hydrateRoot(
+        element,
+        <form ref={ref}>
+          <input type="hidden" name="a" value="A" ref={a} />
+          <input type="submit" name="b" value="B" ref={b} />
+          <button name="c" value="C" ref={c} />
+        </form>,
+      );
+    });
+
+    // The content should not have been client rendered.
+    expect(ref.current).toBe(form);
+
+    expect(a.current.name).toBe('a');
+    expect(a.current.value).toBe('A');
+    expect(b.current.name).toBe('b');
+    expect(b.current.value).toBe('B');
+    expect(c.current.name).toBe('c');
+    expect(c.current.value).toBe('C');
+  });
 });

filename: packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js
patch: @@ -31,9 +31,6 @@ export const getNextHydratableSibling = shim;
 export const getFirstHydratableChild = shim;
 export const getFirstHydratableChildWithinContainer = shim;
 export const getFirstHydratableChildWithinSuspenseInstance = shim;
-export const shouldSkipHydratableForInstance = shim;
-export const shouldSkipHydratableForTextInstance = shim;
-export const shouldSkipHydratableForSuspenseInstance = shim;
 export const canHydrateInstance = shim;
 export const canHydrateTextInstance = shim;
 export const canHydrateSuspenseInstance = shim;

filename: packages/react-reconciler/src/ReactFiberHydrationContext.js
patch: @@ -74,9 +74,6 @@ import {
   didNotFindHydratableTextInstance,
   didNotFindHydratableSuspenseInstance,
   resolveSingletonInstance,
-  shouldSkipHydratableForInstance,
-  shouldSkipHydratableForTextInstance,
-  shouldSkipHydratableForSuspenseInstance,
   canHydrateInstance,
   canHydrateTextInstance,
   canHydrateSuspenseInstance,
@@ -355,6 +352,7 @@ function tryHydrateInstance(fiber: Fiber, nextInstance: any) {
     nextInstance,
     fiber.type,
     fiber.pendingProps,
+    rootOrSingletonContext,
   );
   if (instance !== null) {
     fiber.stateNode = (instance: Instance);
@@ -369,7 +367,11 @@ function tryHydrateInstance(fiber: Fiber, nextInstance: any) {
 function tryHydrateText(fiber: Fiber, nextInstance: any) {
   // fiber is a HostText Fiber
   const text = fiber.pendingProps;
-  const textInstance = canHydrateTextInstance(nextInstance, text);
+  const textInstance = canHydrateTextInstance(
+    nextInstance,
+    text,
+    rootOrSingletonContext,
+  );
   if (textInstance !== null) {
     fiber.stateNode = (textInstance: TextInstance);
     hydrationParentFiber = fiber;
@@ -382,7 +384,10 @@ function tryHydrateText(fiber: Fiber, nextInstance: any) {
 
 function tryHydrateSuspense(fiber: Fiber, nextInstance: any) {
   // fiber is a SuspenseComponent Fiber
-  const suspenseInstance = canHydrateSuspenseInstance(nextInstance);
+  const suspenseInstance = canHydrateSuspenseInstance(
+    nextInstance,
+    rootOrSingletonContext,
+  );
   if (suspenseInstance !== null) {
     const suspenseState: SuspenseState = {
       dehydrated: suspenseInstance,
@@ -441,44 +446,6 @@ function claimHydratableSingleton(fiber: Fiber): void {
   }
 }
 
-function advanceToFirstAttemptableInstance(fiber: Fiber) {
-  // fiber is HostComponent Fiber
-  while (
-    nextHydratableInstance &&
-    shouldSkipHydratableForInstance(
-      nextHydratableInstance,
-      fiber.type,
-      fiber.pendingProps,
-    )
-  ) {
-    // Flow doesn't understand that inside this block nextHydratableInstance is not null
-    const instance: HydratableInstance = (nextHydratableInstance: any);
-    nextHydratableInstance = getNextHydratableSibling(instance);
-  }
-}
-
-function advanceToFirstAttemptableTextInstance() {
-  while (
-    nextHydratableInstance &&
-    shouldSkipHydratableForTextInstance(nextHydratableInstance)
-  ) {
-    // Flow doesn't understand that inside this block nextHydratableInstance is not null
-    const instance: HydratableInstance = (nextHydratableInstance: any);
-    nextHydratableInstance = getNextHydratableSibling(instance);
-  }
-}
-
-function advanceToFirstAttemptableSuspenseInstance() {
-  while (
-    nextHydratableInstance &&
-    shouldSkipHydratableForSuspenseInstance(nextHydratableInstance)
-  ) {
-    // Flow doesn't understand that inside this block nextHydratableInstance is not null
-    const instance: HydratableInstance = (nextHydratableInstance: any);
-    nextHydratableInstance = getNextHydratableSibling(instance);
-  }
-}
-
 function tryToClaimNextHydratableInstance(fiber: Fiber): void {
   if (!isHydrating) {
     return;
@@ -493,10 +460,6 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
     }
   }
   const initialInstance = nextHydratableInstance;
-  if (rootOrSingletonContext) {
-    // We may need to skip past certain nodes in these contexts
-    advanceToFirstAttemptableInstance(fiber);
-  }
   const nextInstance = nextHydratableInstance;
   if (!nextInstance) {
     if (shouldClientRenderOnMismatch(fiber)) {
@@ -521,10 +484,6 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
     // might be flawed or unnecessary.
     nextHydratableInstance = getNextHydratableSibling(nextInstance);
     const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
-    if (rootOrSingletonContext) {
-      // We may need to skip past certain nodes in these contexts
-      advanceToFirstAttemptableInstance(fiber);
-    }
     if (
       !nextHydratableInstance ||
       !tryHydrateInstance(fiber, nextHydratableInstance)
@@ -552,12 +511,6 @@ function tryToClaimNextHydratableTextInstance(fiber: Fiber): void {
   const isHydratable = isHydratableText(text);
 
   const initialInstance = nextHydratableInstance;
-  if (rootOrSingletonContext && isHydratable) {
-    // We may need to skip past certain nodes in these contexts.
-    // We don't skip if the text is not hydratable because we know no hydratables
-    // exist which could match this Fiber
-    advanceToFirstAttemptableTextInstance();
-  }
   const nextInstance = nextHydratableInstance;
   if (!nextInstance || !isHydratable) {
     // We exclude non hydrabable text because we know there are no matching hydratables.
@@ -585,11 +538,6 @@ function tryToClaimNextHydratableTextInstance(fiber: Fiber): void {
     nextHydratableInstance = getNextHydratableSibling(nextInstance);
     const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
 
-    if (rootOrSingletonContext && isHydratable) {
-      // We may need to skip past certain nodes in these contexts
-      advanceToFirstAttemptableTextInstance();
-    }
-
     if (
       !nextHydratableInstance ||
       !tryHydrateText(fiber, nextHydratableInstance)
@@ -614,10 +562,6 @@ function tryToClaimNextHydratableSuspenseInstance(fiber: Fiber): void {
     return;
   }
   const initialInstance = nextHydratableInstance;
-  if (rootOrSingletonContext) {
-    // We may need to skip past certain nodes in these contexts
-    advanceToFirstAttemptableSuspenseInstance();
-  }
   const nextInstance = nextHydratableInstance;
   if (!nextInstance) {
     if (shouldClientRenderOnMismatch(fiber)) {
@@ -643,11 +587,6 @@ function tryToClaimNextHydratableSuspenseInstance(fiber: Fiber): void {
     nextHydratableInstance = getNextHydratableSibling(nextInstance);
     const prevHydrationParentFiber: Fiber = (hydrationParentFiber: any);
 
-    if (rootOrSingletonContext) {
-      // We may need to skip past certain nodes in these contexts
-      advanceToFirstAttemptableSuspenseInstance();
-    }
-
     if (
       !nextHydratableInstance ||
       !tryHydrateSuspense(fiber, nextHydratableInstance)
@@ -863,7 +802,8 @@ function popHydrationState(fiber: Fiber): boolean {
       fiber.tag !== HostSingleton &&
       !(
         fiber.tag === HostComponent &&
-        shouldSetTextContent(fiber.type, fiber.memoizedProps)
+        (!shouldDeleteUnhydratedTailInstances(fiber.type) ||
+          shouldSetTextContent(fiber.type, fiber.memoizedProps))
       )
     ) {
       shouldClear = true;

filename: packages/react-reconciler/src/forks/ReactFiberConfig.custom.js
patch: @@ -149,12 +149,6 @@ export const getFirstHydratableChildWithinContainer =
   $$$config.getFirstHydratableChildWithinContainer;
 export const getFirstHydratableChildWithinSuspenseInstance =
   $$$config.getFirstHydratableChildWithinSuspenseInstance;
-export const shouldSkipHydratableForInstance =
-  $$$config.shouldSkipHydratableForInstance;
-export const shouldSkipHydratableForTextInstance =
-  $$$config.shouldSkipHydratableForTextInstance;
-export const shouldSkipHydratableForSuspenseInstance =
-  $$$config.shouldSkipHydratableForSuspenseInstance;
 export const canHydrateInstance = $$$config.canHydrateInstance;
 export const canHydrateTextInstance = $$$config.canHydrateTextInstance;
 export const canHydrateSuspenseInstance = $$$config.canHydrateSuspenseInstance;