From 361d5a6d5055ddb588d9e0a6ff7bf3224b38222d Mon Sep 17 00:00:00 2001 From: sebmarkbage Date: Thu, 21 Mar 2024 23:56:39 +0000 Subject: [PATCH] [Fizz] Recover from errors thrown by progressive enhancement form generation (#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
Error message
; } } ``` ```js const [errors, formAction] = useActionState(action); return
{errors}
; ``` 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 [fee786a057774ab687aff765345dd86fce534ab2](https://github.com/facebook/react/commit/fee786a057774ab687aff765345dd86fce534ab2) --- compiled/facebook-www/REVISION | 2 +- compiled/facebook-www/React-prod.modern.js | 2 +- .../facebook-www/React-profiling.modern.js | 2 +- .../ReactDOMServer-dev.classic.js | 49 ++++-- .../facebook-www/ReactDOMServer-dev.modern.js | 49 ++++-- .../ReactDOMServer-prod.classic.js | 157 ++++++++++-------- .../ReactDOMServer-prod.modern.js | 157 ++++++++++-------- .../ReactDOMServerStreaming-dev.modern.js | 47 +++++- .../ReactDOMServerStreaming-prod.modern.js | 115 +++++++------ .../__test_utils__/ReactAllWarnings.js | 1 + 10 files changed, 360 insertions(+), 221 deletions(-) diff --git a/compiled/facebook-www/REVISION b/compiled/facebook-www/REVISION index 5732366b99979..15a90d3c190bd 100644 --- a/compiled/facebook-www/REVISION +++ b/compiled/facebook-www/REVISION @@ -1 +1 @@ -8ef14cf24219addedca3607dabb3bef37fb2e013 +fee786a057774ab687aff765345dd86fce534ab2 diff --git a/compiled/facebook-www/React-prod.modern.js b/compiled/facebook-www/React-prod.modern.js index 240a563bac11f..37aa4ba7bc345 100644 --- a/compiled/facebook-www/React-prod.modern.js +++ b/compiled/facebook-www/React-prod.modern.js @@ -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"; diff --git a/compiled/facebook-www/React-profiling.modern.js b/compiled/facebook-www/React-profiling.modern.js index 5b3e5016b07b2..151ad762ca352 100644 --- a/compiled/facebook-www/React-profiling.modern.js +++ b/compiled/facebook-www/React-profiling.modern.js @@ -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 && diff --git a/compiled/facebook-www/ReactDOMServer-dev.classic.js b/compiled/facebook-www/ReactDOMServer-dev.classic.js index 9bbd6cd3f1c8b..9eb5d7f6a8432 100644 --- a/compiled/facebook-www/ReactDOMServer-dev.classic.js +++ b/compiled/facebook-www/ReactDOMServer-dev.classic.js @@ -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"); @@ -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, @@ -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; @@ -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; diff --git a/compiled/facebook-www/ReactDOMServer-dev.modern.js b/compiled/facebook-www/ReactDOMServer-dev.modern.js index 3a3ff0ef7dd65..fef89b11dcf6b 100644 --- a/compiled/facebook-www/ReactDOMServer-dev.modern.js +++ b/compiled/facebook-www/ReactDOMServer-dev.modern.js @@ -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"); @@ -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, @@ -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; @@ -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; diff --git a/compiled/facebook-www/ReactDOMServer-prod.classic.js b/compiled/facebook-www/ReactDOMServer-prod.classic.js index 5885bc33c8f64..1da9376d141bb 100644 --- a/compiled/facebook-www/ReactDOMServer-prod.classic.js +++ b/compiled/facebook-www/ReactDOMServer-prod.classic.js @@ -434,10 +434,6 @@ function pushStringAttribute(target, name, value) { "boolean" !== typeof value && target.push(" ", name, '="', escapeTextForBrowser(value), '"'); } -function makeFormFieldPrefix(resumableState) { - var id = resumableState.nextFormID++; - return resumableState.idPrefix + id; -} var actionJavaScriptURL = escapeTextForBrowser( "javascript:throw new Error('React form unexpectedly submitted.')" ); @@ -448,6 +444,19 @@ function pushAdditionalFormField(value, key) { pushStringAttribute(this, "value", value); this.push("/>"); } +function getCustomFormFields(resumableState, formAction) { + if ("function" === typeof formAction.$$FORM_ACTION) { + var id = resumableState.nextFormID++; + resumableState = resumableState.idPrefix + id; + try { + return formAction.$$FORM_ACTION(resumableState); + } catch (x) { + if ("object" === typeof x && null !== x && "function" === typeof x.then) + throw x; + } + } + return null; +} function pushFormActionAttribute( target, resumableState, @@ -459,19 +468,19 @@ function pushFormActionAttribute( name ) { var formData = null; - "function" === typeof formAction && - ("function" === typeof formAction.$$FORM_ACTION - ? ((formEncType = makeFormFieldPrefix(resumableState)), - (resumableState = formAction.$$FORM_ACTION(formEncType)), - (name = resumableState.name), - (formAction = resumableState.action || ""), - (formEncType = resumableState.encType), - (formMethod = resumableState.method), - (formTarget = resumableState.target), - (formData = resumableState.data)) + if ("function" === typeof formAction) { + var customFields = getCustomFormFields(resumableState, formAction); + null !== customFields + ? ((name = customFields.name), + (formAction = customFields.action || ""), + (formEncType = customFields.encType), + (formMethod = customFields.method), + (formTarget = customFields.target), + (formData = customFields.data)) : (target.push(" ", "formAction", '="', actionJavaScriptURL, '"'), (formTarget = formMethod = formEncType = formAction = name = null), - injectFormReplayingRuntime(resumableState, renderState))); + injectFormReplayingRuntime(resumableState, renderState)); + } null != name && pushAttribute(target, "name", name); null != formAction && pushAttribute(target, "formAction", formAction); null != formEncType && pushAttribute(target, "formEncType", formEncType); @@ -1068,7 +1077,7 @@ function pushStartInstance( : null !== defaultValue$jscomp$0 && pushAttribute(target$jscomp$0, "value", defaultValue$jscomp$0); target$jscomp$0.push("/>"); - null !== formData && + null != formData && formData.forEach(pushAdditionalFormField, target$jscomp$0); return null; case "button": @@ -1126,7 +1135,7 @@ function pushStartInstance( name$jscomp$0 ); target$jscomp$0.push(">"); - null !== formData$jscomp$0 && + null != formData$jscomp$0 && formData$jscomp$0.forEach(pushAdditionalFormField, target$jscomp$0); pushInnerHTML(target$jscomp$0, innerHTML$jscomp$2, children$jscomp$3); if ("string" === typeof children$jscomp$3) { @@ -1176,24 +1185,32 @@ function pushStartInstance( } var formData$jscomp$1 = null, formActionName = null; - if ("function" === typeof formAction$jscomp$1) - if ("function" === typeof formAction$jscomp$1.$$FORM_ACTION) { - var prefix$9 = makeFormFieldPrefix(resumableState), - customFields = formAction$jscomp$1.$$FORM_ACTION(prefix$9); - formAction$jscomp$1 = customFields.action || ""; - formEncType$jscomp$1 = customFields.encType; - formMethod$jscomp$1 = customFields.method; - formTarget$jscomp$1 = customFields.target; - formData$jscomp$1 = customFields.data; - formActionName = customFields.name; - } else - target$jscomp$0.push(" ", "action", '="', actionJavaScriptURL, '"'), + if ("function" === typeof formAction$jscomp$1) { + var customFields = getCustomFormFields( + resumableState, + formAction$jscomp$1 + ); + null !== customFields + ? ((formAction$jscomp$1 = customFields.action || ""), + (formEncType$jscomp$1 = customFields.encType), + (formMethod$jscomp$1 = customFields.method), + (formTarget$jscomp$1 = customFields.target), + (formData$jscomp$1 = customFields.data), + (formActionName = customFields.name)) + : (target$jscomp$0.push( + " ", + "action", + '="', + actionJavaScriptURL, + '"' + ), (formTarget$jscomp$1 = formMethod$jscomp$1 = formEncType$jscomp$1 = formAction$jscomp$1 = null), - injectFormReplayingRuntime(resumableState, renderState); + injectFormReplayingRuntime(resumableState, renderState)); + } null != formAction$jscomp$1 && pushAttribute(target$jscomp$0, "action", formAction$jscomp$1); null != formEncType$jscomp$1 && @@ -1207,7 +1224,7 @@ function pushStartInstance( (target$jscomp$0.push('"), - null !== formData$jscomp$1 && + null != formData$jscomp$1 && formData$jscomp$1.forEach(pushAdditionalFormField, target$jscomp$0)); pushInnerHTML(target$jscomp$0, innerHTML$jscomp$3, children$jscomp$4); if ("string" === typeof children$jscomp$4) { @@ -1309,10 +1326,10 @@ function pushStartInstance( styleQueue.sheets.set(href, resource); hoistableState && hoistableState.stylesheets.add(resource); } else if (styleQueue) { - var resource$10 = styleQueue.sheets.get(href); - resource$10 && + var resource$9 = styleQueue.sheets.get(href); + resource$9 && hoistableState && - hoistableState.stylesheets.add(resource$10); + hoistableState.stylesheets.add(resource$9); } textEmbedded && target$jscomp$0.push("\x3c!-- --\x3e"); JSCompiler_inline_result$jscomp$3 = null; @@ -2604,16 +2621,16 @@ function createRenderState(resumableState, generateStaticMarkup) { "\x3c/script>" ); bootstrapScriptContent = idPrefix + "P:"; - var JSCompiler_object_inline_segmentPrefix_1595 = idPrefix + "S:"; + var JSCompiler_object_inline_segmentPrefix_1596 = idPrefix + "S:"; idPrefix += "B:"; - var JSCompiler_object_inline_preconnects_1609 = new Set(), - JSCompiler_object_inline_fontPreloads_1610 = new Set(), - JSCompiler_object_inline_highImagePreloads_1611 = new Set(), - JSCompiler_object_inline_styles_1612 = new Map(), - JSCompiler_object_inline_bootstrapScripts_1613 = new Set(), - JSCompiler_object_inline_scripts_1614 = new Set(), - JSCompiler_object_inline_bulkPreloads_1615 = new Set(), - JSCompiler_object_inline_preloads_1616 = { + var JSCompiler_object_inline_preconnects_1610 = new Set(), + JSCompiler_object_inline_fontPreloads_1611 = new Set(), + JSCompiler_object_inline_highImagePreloads_1612 = new Set(), + JSCompiler_object_inline_styles_1613 = new Map(), + JSCompiler_object_inline_bootstrapScripts_1614 = new Set(), + JSCompiler_object_inline_scripts_1615 = new Set(), + JSCompiler_object_inline_bulkPreloads_1616 = new Set(), + JSCompiler_object_inline_preloads_1617 = { images: new Map(), stylesheets: new Map(), scripts: new Map(), @@ -2650,7 +2667,7 @@ function createRenderState(resumableState, generateStaticMarkup) { scriptConfig.moduleScriptResources[href] = null; scriptConfig = []; pushLinkImpl(scriptConfig, props); - JSCompiler_object_inline_bootstrapScripts_1613.add(scriptConfig); + JSCompiler_object_inline_bootstrapScripts_1614.add(scriptConfig); bootstrapChunks.push('