Skip to content

Commit

Permalink
[Fizz] Recover from errors thrown by progressive enhancement form gen…
Browse files Browse the repository at this point in the history
…eration (#28611)

This a follow up to #28564. It's alternative to #28609 which takes
#28610 into account.

It used to be possible to return JSX from an action with
`useActionState`.

```js
async function action(errors, payload) {
  "use server";
  try {
    ...
  } catch (x) {
    return <div>Error message</div>;
  }
}
```

```js
const [errors, formAction] = useActionState(action);
return <div>{errors}</div>;
```

Returning JSX from an action is itself not anything problematic. It's
that it also has to return the previous state to the action reducer
again that's the problem. When this happens we accidentally could
serialize an Element back to the server.

I fixed this in #28564 so it's now blocked if you don't have a temporary
reference set.

However, you can't have that for the progressive enhancement case. The
reply is eagerly encoded as part of the SSR render. Typically you
wouldn't have these in the initial state so the common case is that they
show up after the first POST back that yields an error and it's only in
the no-JS case where this happens so it's hard to discover.

As noted in #28609 there's a security implication with allowing elements
to be sent across this kind of payload, so we can't just make it work.

When an error happens during SSR our general policy is to try to recover
on the client instead. After all, SSR is mainly a perf optimization in
React terms and it's not primarily intended for a no JS solution.

This PR takes the approach that if we fail to generate the progressive
enhancement payload. I.e. if the serialization of previous state /
closures throw. Then we fallback to the replaying semantics just client
actions instead which will succeed.

The effect of this is that this pattern mostly just works:

- First render in the typical case doesn't have any JSX in it so it just
renders a progressive enhanced form.
- If JS fails to hydrate or you click early we do a form POST. If that
hits an error and it tries to render it using JSX, then the new page
will render successfully - however this time with a Replaying form
instead.
- If you try to submit the form again it'll have to be using JS.

Meaning if you use JSX as the error return value of form state and you
make a first attempt that fails, then no JS won't work because either
the first or second attempt has to hydrate.

We have ideas for potentially optimizing away serializing unused
arguments like if you don't actually use previous state which would also
solve it but it wouldn't cover all cases such as if it was deeply nested
in complex state.

Another approach that I considered was to poison the prev state if you
passed an element back but let it through to the action but if you try
to render the poisoned value, it wouldn't work. The downside of this is
when to error. Because in the progressive enhancement case it wouldn't
error early but when you actually try to invoke it at which point it
would be too late to fallback to client replaying. It would probably
have to always error even on the client which is unfortunate since this
mostly just works as long as it hydrates.

DiffTrain build for [fee786a](fee786a)
  • Loading branch information
sebmarkbage committed Mar 21, 2024
1 parent 6e5eaa6 commit 361d5a6
Show file tree
Hide file tree
Showing 10 changed files with 360 additions and 221 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8ef14cf24219addedca3607dabb3bef37fb2e013
fee786a057774ab687aff765345dd86fce534ab2
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -625,4 +625,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-94e2c94e";
exports.version = "18.3.0-www-modern-e739dc06";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-1d6a3285";
exports.version = "18.3.0-www-modern-b2de0947";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
49 changes: 39 additions & 10 deletions compiled/facebook-www/ReactDOMServer-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-classic-c69e4b0a";
var ReactVersion = "18.3.0-www-classic-1ff2dafc";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -3170,12 +3170,45 @@ if (__DEV__) {
}

function pushAdditionalFormFields(target, formData) {
if (formData !== null) {
if (formData != null) {
// $FlowFixMe[prop-missing]: FormData has forEach.
formData.forEach(pushAdditionalFormField, target);
}
}

function getCustomFormFields(resumableState, formAction) {
var customAction = formAction.$$FORM_ACTION;

if (typeof customAction === "function") {
var prefix = makeFormFieldPrefix(resumableState);

try {
return formAction.$$FORM_ACTION(prefix);
} catch (x) {
if (
typeof x === "object" &&
x !== null &&
typeof x.then === "function"
) {
// Rethrow suspense.
throw x;
} // If we fail to encode the form action for progressive enhancement for some reason,
// fallback to trying replaying on the client instead of failing the page. It might
// work there.

{
// TODO: Should this be some kind of recoverable error?
error(
"Failed to serialize an action for progressive enhancement:\n%s",
x
);
}
}
}

return null;
}

function pushFormActionAttribute(
target,
resumableState,
Expand Down Expand Up @@ -3222,13 +3255,11 @@ if (__DEV__) {
}
}

var customAction = formAction.$$FORM_ACTION;
var customFields = getCustomFormFields(resumableState, formAction);

if (typeof customAction === "function") {
if (customFields !== null) {
// 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.
var prefix = makeFormFieldPrefix(resumableState);
var customFields = formAction.$$FORM_ACTION(prefix);
name = customFields.name;
formAction = customFields.action || "";
formEncType = customFields.encType;
Expand Down Expand Up @@ -4088,13 +4119,11 @@ if (__DEV__) {
}
}

var customAction = formAction.$$FORM_ACTION;
var customFields = getCustomFormFields(resumableState, formAction);

if (typeof customAction === "function") {
if (customFields !== null) {
// 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.
var prefix = makeFormFieldPrefix(resumableState);
var customFields = formAction.$$FORM_ACTION(prefix);
formAction = customFields.action || "";
formEncType = customFields.encType;
formMethod = customFields.method;
Expand Down
49 changes: 39 additions & 10 deletions compiled/facebook-www/ReactDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-modern-ac7e39a5";
var ReactVersion = "18.3.0-www-modern-bfd3973a";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -3170,12 +3170,45 @@ if (__DEV__) {
}

function pushAdditionalFormFields(target, formData) {
if (formData !== null) {
if (formData != null) {
// $FlowFixMe[prop-missing]: FormData has forEach.
formData.forEach(pushAdditionalFormField, target);
}
}

function getCustomFormFields(resumableState, formAction) {
var customAction = formAction.$$FORM_ACTION;

if (typeof customAction === "function") {
var prefix = makeFormFieldPrefix(resumableState);

try {
return formAction.$$FORM_ACTION(prefix);
} catch (x) {
if (
typeof x === "object" &&
x !== null &&
typeof x.then === "function"
) {
// Rethrow suspense.
throw x;
} // If we fail to encode the form action for progressive enhancement for some reason,
// fallback to trying replaying on the client instead of failing the page. It might
// work there.

{
// TODO: Should this be some kind of recoverable error?
error(
"Failed to serialize an action for progressive enhancement:\n%s",
x
);
}
}
}

return null;
}

function pushFormActionAttribute(
target,
resumableState,
Expand Down Expand Up @@ -3222,13 +3255,11 @@ if (__DEV__) {
}
}

var customAction = formAction.$$FORM_ACTION;
var customFields = getCustomFormFields(resumableState, formAction);

if (typeof customAction === "function") {
if (customFields !== null) {
// 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.
var prefix = makeFormFieldPrefix(resumableState);
var customFields = formAction.$$FORM_ACTION(prefix);
name = customFields.name;
formAction = customFields.action || "";
formEncType = customFields.encType;
Expand Down Expand Up @@ -4088,13 +4119,11 @@ if (__DEV__) {
}
}

var customAction = formAction.$$FORM_ACTION;
var customFields = getCustomFormFields(resumableState, formAction);

if (typeof customAction === "function") {
if (customFields !== null) {
// 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.
var prefix = makeFormFieldPrefix(resumableState);
var customFields = formAction.$$FORM_ACTION(prefix);
formAction = customFields.action || "";
formEncType = customFields.encType;
formMethod = customFields.method;
Expand Down
Loading

0 comments on commit 361d5a6

Please sign in to comment.