Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
May 1, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ export function shouldDeleteUnhydratedTailInstances(
return (
(enableHostSingletons ||
(parentType !== 'head' && parentType !== 'body')) &&
(!enableFormActions || parentType !== 'form')
(!enableFormActions || (parentType !== 'form' && parentType !== 'button'))
);
}

Expand Down
216 changes: 158 additions & 58 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -677,6 +693,36 @@ const actionJavaScriptURL = stringToPrecomputedChunk(
),
);

const startHiddenInputChunk = stringToPrecomputedChunk('<input type="hidden"');

function pushAdditionalFormField(
this: Array<Chunk | PrecomputedChunk>,
value: string | File,
key: string,
): void {
const target: Array<Chunk | PrecomputedChunk> = 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<Chunk | PrecomputedChunk>,
formData: null | FormData,
) {
if (formData !== null) {
// $FlowFixMe[prop-missing]: FormData has forEach.
formData.forEach(pushAdditionalFormField, target);
}
}

function pushFormActionAttribute(
target: Array<Chunk | PrecomputedChunk>,
responseState: ResponseState,
Expand All @@ -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__) {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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__) {
Expand All @@ -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.
Expand Down Expand Up @@ -1510,7 +1601,7 @@ function pushInput(
}
}

pushFormActionAttribute(
const formData = pushFormActionAttribute(
target,
responseState,
formAction,
Expand Down Expand Up @@ -1561,6 +1652,10 @@ function pushInput(
}

target.push(endOfStartTagSelfClosing);

// We place any additional hidden form fields after the input.
pushAdditionalFormFields(target, formData);

return null;
}

Expand Down Expand Up @@ -1628,7 +1723,7 @@ function pushStartButton(
}
}

pushFormActionAttribute(
const formData = pushFormActionAttribute(
target,
responseState,
formAction,
Expand All @@ -1639,13 +1734,18 @@ 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.
// TODO: Remove this special case after the general optimization is in place.
target.push(stringToChunk(encodeHTMLTextNode(children)));
return null;
}

return children;
}

Expand Down
Loading