From 559e83aebb2026035d47aa0ebf842f78d4cd6757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 1 May 2023 16:01:14 -0400 Subject: [PATCH] [Fizz] Allow an action provide a custom set of props to use for progressive 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. --- .../src/client/ReactDOMComponent.js | 14 +- .../src/client/ReactFiberConfigDOM.js | 2 +- .../src/server/ReactFizzConfigDOM.js | 216 +++++++++++++----- .../src/__tests__/ReactDOMFizzForm-test.js | 142 +++++++++++- scripts/error-codes/codes.json | 3 +- 5 files changed, 302 insertions(+), 75 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 2603d8db35..6454ee7c2d 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/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; diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 6a4a55a2ad..363048f070 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/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')) ); } diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 646e61608a..ee521d9336 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -668,6 +668,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( @@ -677,6 +693,36 @@ const actionJavaScriptURL = stringToPrecomputedChunk( ), ); +const startHiddenInputChunk = stringToPrecomputedChunk(', + value: string | File, + key: string, +): void { + const target: Array = 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, + formData: null | FormData, +) { + if (formData !== null) { + // $FlowFixMe[prop-missing]: FormData has forEach. + formData.forEach(pushAdditionalFormField, target); + } +} + function pushFormActionAttribute( target: Array, responseState: ResponseState, @@ -685,7 +731,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__) { @@ -714,37 +761,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 = formAction.$$FORM_ACTION(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( @@ -1366,6 +1431,8 @@ function pushStartForm( } } + let formData = null; + let formActionName = null; if (enableFormActions && typeof formAction === 'function') { // Function form actions cannot control the form properties if (__DEV__) { @@ -1388,36 +1455,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 = formAction.$$FORM_ACTION(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. @@ -1510,7 +1601,7 @@ function pushInput( } } - pushFormActionAttribute( + const formData = pushFormActionAttribute( target, responseState, formAction, @@ -1561,6 +1652,10 @@ function pushInput( } target.push(endOfStartTagSelfClosing); + + // We place any additional hidden form fields after the input. + pushAdditionalFormFields(target, formData); + return null; } @@ -1628,7 +1723,7 @@ function pushStartButton( } } - pushFormActionAttribute( + const formData = pushFormActionAttribute( target, responseState, formAction, @@ -1639,6 +1734,10 @@ 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. @@ -1646,6 +1745,7 @@ function pushStartButton( target.push(stringToChunk(encodeHTMLTextNode(children))); return null; } + return children; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js index 5d3f3ea004..eef6472098 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js @@ -183,7 +183,7 @@ describe('ReactDOMFizzForm', () => { }); // @gate enableFormActions || !__DEV__ - it('should warn when passing a string during SSR and function during hydration', async () => { + it('should ideally warn when passing a string during SSR and function during hydration', async () => { function action(formData) {} function App({isClient}) { return ( @@ -195,13 +195,10 @@ describe('ReactDOMFizzForm', () => { const stream = await ReactDOMServer.renderToReadableStream(); await readIntoContainer(stream); - await expect(async () => { - await act(async () => { - ReactDOMClient.hydrateRoot(container, ); - }); - }).toErrorDev( - 'Prop `action` did not match. Server: "action" Client: "function action(formData) {}"', - ); + // This should ideally warn because only the client provides a function that doesn't line up. + await act(async () => { + ReactDOMClient.hydrateRoot(container, ); + }); }); // @gate enableFormActions || !__DEV__ @@ -472,4 +469,133 @@ describe('ReactDOMFizzForm', () => { }); expect(container.textContent).toBe('hi'); }); + + // @gate enableFormActions + it('can provide a custom action on the server for actions', async () => { + const ref = React.createRef(); + let foo; + + function action(formData) { + foo = formData.get('foo'); + } + action.$$FORM_ACTION = function (identifierPrefix) { + const extraFields = new FormData(); + extraFields.append(identifierPrefix + 'hello', 'world'); + return { + action: this.name, + name: identifierPrefix, + method: 'POST', + encType: 'multipart/form-data', + target: 'self', + data: extraFields, + }; + }; + function App() { + return ( +
+ +
+ ); + } + + const stream = await ReactDOMServer.renderToReadableStream(); + await readIntoContainer(stream); + + const form = container.firstChild; + expect(form.getAttribute('action')).toBe('action'); + expect(form.getAttribute('method')).toBe('POST'); + expect(form.getAttribute('enctype')).toBe('multipart/form-data'); + expect(form.getAttribute('target')).toBe('self'); + const formActionName = form.firstChild.getAttribute('name'); + expect( + container + .querySelector('input[name="' + formActionName + 'hello"]') + .getAttribute('value'), + ).toBe('world'); + + await act(async () => { + ReactDOMClient.hydrateRoot(container, ); + }); + + submit(ref.current); + + expect(foo).toBe('bar'); + }); + + // @gate enableFormActions + it('can provide a custom action on buttons the server for actions', async () => { + const inputRef = React.createRef(); + const buttonRef = React.createRef(); + let foo; + + function action(formData) { + foo = formData.get('foo'); + } + action.$$FORM_ACTION = function (identifierPrefix) { + const extraFields = new FormData(); + extraFields.append(identifierPrefix + 'hello', 'world'); + return { + action: this.name, + name: identifierPrefix, + method: 'POST', + encType: 'multipart/form-data', + target: 'self', + data: extraFields, + }; + }; + function App() { + return ( +
+ + +