Allow forms to skip hydration of hidden inputs #26735

sebmarkbage · 2023-04-28T05:41:38Z

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.

These config methods can already skip passed and it allows us to unify these into a single check.

This lets us reuse the nodeType check.

sebmarkbage · 2023-04-28T05:43:07Z

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

+      // Match
+      if (
+        enableFormActions &&
+        type === 'input' &&


This could just go into the switch below if we didn't bother checking the inRootOrSingleton context. Which probably makes sense since the other rules really apply to all context. It's just maybe worth for perf - especially checking isMarkedHoistable unnecessarily.

yeah maybe we just ignore the root/singleton context. Questioning that in my other comment

sebmarkbage · 2023-04-28T05:43:35Z

I could argue that maybe we should just never throw in prod for extra nodes to simplify the rules and code. The main mechanism we rely on is actually text nodes anyway since we don't look at the attributes. Text nodes typically cover mistakes like keys or wrong cases not lining up in production. As long as we do find all nodes, it's at least possible to continue.

react-sizebot · 2023-04-28T05:45:47Z

Comparing: 540bab0...b2b497c

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	164.02 kB	164.03 kB	+0.01%	51.71 kB	51.71 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.08%	170.32 kB	170.45 kB	+0.08%	53.62 kB	53.67 kB
facebook-www/ReactDOM-prod.classic.js	=	566.81 kB	566.39 kB	=	100.13 kB	100.07 kB
facebook-www/ReactDOM-prod.modern.js	=	550.55 kB	550.13 kB	=	97.33 kB	97.25 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-reconciler/cjs/react-reconciler.development.js	=	914.08 kB	911.74 kB	=	195.67 kB	195.36 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js	=	899.41 kB	897.07 kB	=	192.54 kB	192.26 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js	=	899.38 kB	897.04 kB	=	192.51 kB	192.23 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.min.js	=	123.43 kB	123.10 kB	=	37.20 kB	37.12 kB
oss-stable/react-reconciler/cjs/react-reconciler.profiling.min.js	=	120.20 kB	119.88 kB	=	36.19 kB	36.09 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.profiling.min.js	=	120.18 kB	119.85 kB	=	36.17 kB	36.07 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js	=	114.42 kB	114.09 kB	=	35.02 kB	34.93 kB
oss-stable/react-reconciler/cjs/react-reconciler.production.min.js	=	111.19 kB	110.86 kB	=	33.96 kB	33.85 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.production.min.js	=	111.17 kB	110.84 kB	=	33.94 kB	33.83 kB

Generated by 🚫 dangerJS against b2b497c

This lets us put extra data to encode actions at the end.

Really this could be limited to just buttons but I just made it a general thing.

gnoff

I'd approve except I think there is a (possibly pre-existing) bug with skipping past suspense nodes.

gnoff · 2023-05-01T15:28:32Z

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

+    if (!inRootOrSingleton || !enableHostSingletons) {
+      return null;
+    }
+    const nextInstance = getNextHydratableSibling(instance);
+    if (nextInstance === null) {
+      return null;
+    }
+    instance = nextInstance;


I think this may be a flaw already but if the hydratable instance is a suspense node but we're trying to hydrate text and we're in the root or a singleton then we'll call getNextHydratableSibling on the comment node which is incorrect. If hydratable is a comment I think we need to return null here

At the same time, does it matter? It's going to be a hydration error anyway, most likely, at least when the Suspense boundary doesn't find its node.

Arguably this is more resilient to non-suspense boundary comments that get inserted. I'm hesitant to "fix" it for that reason. Also, note that this is the same as the previous case. Only in inRootOrSingleton scope.

makes sense, marked approved

gnoff · 2023-05-01T15:34:20Z

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

+  return (
+    (enableHostSingletons ||
+      (parentType !== 'head' && parentType !== 'body')) &&
+    (!enableFormActions || parentType !== 'form')
+  );


This function is doing double duty as the signal for not clearing forms and as a signal to not clear head and body when singletons flag is turned off. We should make it two separate functions so it is clearer we can delete the legacy one once the singletons flag lands

gnoff · 2023-05-01T15:44:15Z

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

-          element.getAttribute('title') !==
-            (anyProps.title == null ? null : anyProps.title)
+    if (element.nodeName.toLowerCase() !== type.toLowerCase()) {
+      if (!inRootOrSingleton || !enableHostSingletons) {


why do we need the host singleton check here? with flag is off we always hit this branch and with it on we skip it at the root level (and at the singleton level) but practically speaking we'll never be at root or singleton level because we'll always be in a form when this counts.

Seems like we can avoid the check altogether and always verify based on nodeName === 'input' or we can just use the inRootOrSingleton check and rely on the fact that when singleton flag is off it turn into an inRoot check

I think this was just an oversight before this PR that this code should've been just completely eliminated with the flag off. Basically there's no loop here without it since it always exists.

gnoff · 2023-05-01T15:49:55Z

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

+      // Match
+      if (
+        enableFormActions &&
+        type === 'input' &&


yeah maybe we just ignore the root/singleton context. Questioning that in my other comment

gnoff · 2023-05-01T15:55:40Z

I could argue that maybe we should just never throw in prod for extra nodes to simplify the rules and code. The main mechanism we rely on is actually text nodes anyway since we don't look at the attributes. Text nodes typically cover mistakes like keys or wrong cases not lining up in production. As long as we do find all nodes, it's at least possible to continue.

Would that simplify much? the tail nodes thing is maybe easy to just apply all the time but could we get away with skipping extra elements in every context? I suppose you're considering Text and Suspense boundaries as anchors at that point

Hmm maybe this is actually good. I kinda like the idea of consistent behavior and we can stop tracking scope

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. DiffTrain build for [67f4fb0](67f4fb0)

…essive enhancement (#26749) Stacked on top of #26735. This allows a framework to add a `$$FORM_ACTION` property to a function. This lets the framework return a set of props to use in place of the function but only during SSR. Effectively, this lets you implement progressive enhancement of form actions using some other way instead of relying on the replay feature. This will be used by RSC on Server References automatically by convention in a follow up, but this mechanism can also be used by other frameworks/libraries.

…essive enhancement (#26749) Stacked on top of #26735. This allows a framework to add a `$$FORM_ACTION` property to a function. This lets the framework return a set of props to use in place of the function but only during SSR. Effectively, this lets you implement progressive enhancement of form actions using some other way instead of relying on the replay feature. This will be used by RSC on Server References automatically by convention in a follow up, but this mechanism can also be used by other frameworks/libraries. DiffTrain build for [559e83a](559e83a)

Includes the following upstream changes: - [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([vercel#26734](facebook/react#26734)) (Samuel Susla) - [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([vercel#26749](facebook/react#26749)) (Sebastian Markbåge) - [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([vercel#26735](facebook/react#26735)) (Sebastian Markbåge) - [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([vercel#26752](facebook/react#26752)) (Josh Story) - [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([vercel#26740](facebook/react#26740)) (Andrew Clark) - [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([vercel#26738](facebook/react#26738)) (Dan Ott) - [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark) - [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story) - [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([vercel#26743](facebook/react#26743)) (Sebastian Silbermann) - [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([vercel#26739](facebook/react#26739)) (Andrew Clark) - [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark)

Includes the following upstream changes: - [b7972822b](https://github.com/facebook/react/commits/b7972822b) useOptimisticState -> useOptimistic ([#26772](facebook/react#26772)) (Andrew Clark) - [388686f29](https://github.com/facebook/react/commits/388686f29) Add "canary" to list of allowed npm dist tags ([#26767](facebook/react#26767)) (Andrew Clark) - [8a25302c6](https://github.com/facebook/react/commits/8a25302c6) fix[dynamic-scripts-injection]: unregister content scripts before registration ([#26765](facebook/react#26765)) (Ruslan Lesiutin) - [2c2476834](https://github.com/facebook/react/commits/2c2476834) Rename "next" prerelease channel to "canary" ([#26761](facebook/react#26761)) (Andrew Clark) - [fa4314841](https://github.com/facebook/react/commits/fa4314841) Remove deprecated workflow key from Circle config ([#26762](facebook/react#26762)) (Andrew Clark) - [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([#26734](facebook/react#26734)) (Samuel Susla) - [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([#26749](facebook/react#26749)) (Sebastian Markbåge) - [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([#26735](facebook/react#26735)) (Sebastian Markbåge) - [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([#26752](facebook/react#26752)) (Josh Story) - [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([#26740](facebook/react#26740)) (Andrew Clark) - [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([#26738](facebook/react#26738)) (Dan Ott) - [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark) - [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story) - [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([#26743](facebook/react#26743)) (Sebastian Silbermann) - [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([#26739](facebook/react#26739)) (Andrew Clark) - [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark) ---------

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.

…essive enhancement (facebook#26749) Stacked on top of facebook#26735. This allows a framework to add a `$$FORM_ACTION` property to a function. This lets the framework return a set of props to use in place of the function but only during SSR. Effectively, this lets you implement progressive enhancement of form actions using some other way instead of relying on the replay feature. This will be used by RSC on Server References automatically by convention in a follow up, but this mechanism can also be used by other frameworks/libraries.

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. DiffTrain build for commit 67f4fb0.

…essive enhancement (#26749) Stacked on top of #26735. This allows a framework to add a `$$FORM_ACTION` property to a function. This lets the framework return a set of props to use in place of the function but only during SSR. Effectively, this lets you implement progressive enhancement of form actions using some other way instead of relying on the replay feature. This will be used by RSC on Server References automatically by convention in a follow up, but this mechanism can also be used by other frameworks/libraries. DiffTrain build for commit 559e83a.

sebmarkbage added 2 commits April 27, 2023 23:29

Move shouldSkip into canHydrate

7b55c14

These config methods can already skip passed and it allows us to unify these into a single check.

Inline shouldSkip into canHydrate

4cbdc87

This lets us reuse the nodeType check.

sebmarkbage requested a review from gnoff April 28, 2023 05:41

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 28, 2023

sebmarkbage commented Apr 28, 2023

View reviewed changes

sebmarkbage force-pushed the skipincan branch 3 times, most recently from 4c5d6f4 to 04e4daa Compare April 28, 2023 15:36

sebmarkbage added 2 commits April 28, 2023 11:49

Filter don't delete tail nodes of forms

62feb18

This lets us put extra data to encode actions at the end.

Allow hidden inputs as extra nodes that we don't match

b2b497c

Really this could be limited to just buttons but I just made it a general thing.

sebmarkbage force-pushed the skipincan branch from 04e4daa to b2b497c Compare April 28, 2023 15:50

sebmarkbage mentioned this pull request May 1, 2023

[Fizz] Allow an action provide a custom set of props to use for progressive enhancement #26749

Merged

gnoff requested changes May 1, 2023

View reviewed changes

gnoff approved these changes May 1, 2023

View reviewed changes

sebmarkbage merged commit 67f4fb0 into facebook:main May 1, 2023

acdlite mentioned this pull request May 3, 2023

Upgrade React to 18.3.0-canary-b7972822b-20230503 vercel/next.js#49158

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow forms to skip hydration of hidden inputs #26735

Allow forms to skip hydration of hidden inputs #26735

sebmarkbage commented Apr 28, 2023 •

edited

Loading

sebmarkbage Apr 28, 2023

gnoff May 1, 2023

sebmarkbage commented Apr 28, 2023

react-sizebot commented Apr 28, 2023 •

edited

Loading

gnoff left a comment

gnoff May 1, 2023

sebmarkbage May 1, 2023

gnoff May 1, 2023

gnoff May 1, 2023

gnoff May 1, 2023

sebmarkbage May 1, 2023

gnoff May 1, 2023

gnoff commented May 1, 2023

Allow forms to skip hydration of hidden inputs #26735

Allow forms to skip hydration of hidden inputs #26735

Conversation

sebmarkbage commented Apr 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebmarkbage commented Apr 28, 2023

react-sizebot commented Apr 28, 2023 • edited Loading

Critical size changes

Significant size changes

gnoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnoff commented May 1, 2023

sebmarkbage commented Apr 28, 2023 •

edited

Loading

react-sizebot commented Apr 28, 2023 •

edited

Loading