Skip to content

Commit

Permalink
Restore server controlled form fields to whatever they should be (#26708
Browse files Browse the repository at this point in the history 
)

Fizz can emit whatever it wants for the SSR version of these fields when
it's a function action so they might not align with what is in the
previous props. Therefore we need to force them to update if we're
updating to a non-function where they might be relevant again.
  • Loading branch information
@sebmarkbage
sebmarkbage authored Apr 23, 2023
1 parent 7ce765e commit 2fa6323
Show file tree
Hide file tree
Showing 3 changed files with 283 additions and 28 deletions.
145 changes: 118 additions & 27 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,68 @@ function setProp(
case 'action':
case 'formAction': {
// TODO: Consider moving these special cases to the form, input and button tags.
if (enableFormActions) {
if (typeof value === 'function') {
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
// because we'll preventDefault, 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.
domElement.setAttribute(
key,
// eslint-disable-next-line no-script-url
"javascript:throw new Error('" +
'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
"consider using form.requestSubmit() instead. If you're trying to use " +
'event.stopPropagation() in a submit event handler, consider also calling ' +
'event.preventDefault().' +
"')",
);
break;
} else if (typeof prevValue === 'function') {
// When we're switching off a Server Action that was originally hydrated.
// The server control these fields during SSR that are now trailing.
// The regular diffing doesn't apply since we compare against the previous props.
// Instead, we need to force them to be set to whatever they should be now.
// This would be a lot cleaner if we did this whole fork in the per-tag approach.
if (key === 'formAction') {
if (tag !== 'input') {
// Setting the name here isn't completely safe for inputs if this is switching
// to become a radio button. In that case we let the tag based override take
// control.
setProp(domElement, tag, 'name', props.name, props, null);
}
setProp(
domElement,
tag,
'formEncType',
props.formEncType,
props,
null,
);
setProp(
domElement,
tag,
'formMethod',
props.formMethod,
props,
null,
);
setProp(
domElement,
tag,
'formTarget',
props.formTarget,
props,
null,
);
} else {
setProp(domElement, tag, 'encType', props.encType, props, null);
setProp(domElement, tag, 'method', props.method, props, null);
setProp(domElement, tag, 'target', props.target, props, null);
}
}
}
if (
value == null ||
(!enableFormActions && typeof value === 'function') ||
Expand All @@ -495,24 +557,6 @@ function setProp(
if (__DEV__) {
validateFormActionInDevelopment(tag, key, value, props);
}
if (enableFormActions && typeof value === 'function') {
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
// because we'll preventDefault, 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.
domElement.setAttribute(
key,
// eslint-disable-next-line no-script-url
"javascript:throw new Error('" +
'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
"consider using form.requestSubmit() instead. If you're trying to use " +
'event.stopPropagation() in a submit event handler, consider also calling ' +
'event.preventDefault().' +
"')",
);
break;
}
// `setAttribute` with objects becomes only `[object]` in IE8/9,
// ('' + value) makes it output the correct toString()-value.
if (__DEV__) {
Expand Down Expand Up @@ -1138,7 +1182,7 @@ export function setInitialProperties(
break;
}
default: {
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
Expand Down Expand Up @@ -1169,7 +1213,7 @@ export function setInitialProperties(
break;
}
default: {
setProp(domElement, tag, propKey, propValue, props);
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
Expand Down Expand Up @@ -1935,7 +1979,14 @@ export function updatePropertiesWithDiff(
break;
}
default: {
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(
domElement,
tag,
propKey,
propValue,
nextProps,
lastProps[propKey],
);
}
}
}
Expand Down Expand Up @@ -2010,7 +2061,14 @@ export function updatePropertiesWithDiff(
}
// defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(
domElement,
tag,
propKey,
propValue,
nextProps,
lastProps[propKey],
);
}
}
}
Expand Down Expand Up @@ -2045,7 +2103,14 @@ export function updatePropertiesWithDiff(
}
// defaultValue is ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(
domElement,
tag,
propKey,
propValue,
nextProps,
lastProps[propKey],
);
}
}
}
Expand All @@ -2066,7 +2131,14 @@ export function updatePropertiesWithDiff(
break;
}
default: {
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(
domElement,
tag,
propKey,
propValue,
nextProps,
lastProps[propKey],
);
}
}
}
Expand Down Expand Up @@ -2105,7 +2177,14 @@ export function updatePropertiesWithDiff(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(
domElement,
tag,
propKey,
propValue,
nextProps,
lastProps[propKey],
);
}
}
}
Expand All @@ -2122,7 +2201,7 @@ export function updatePropertiesWithDiff(
propKey,
propValue,
nextProps,
null,
lastProps[propKey],
);
}
return;
Expand All @@ -2134,7 +2213,7 @@ export function updatePropertiesWithDiff(
for (let i = 0; i < updatePayload.length; i += 2) {
const propKey = updatePayload[i];
const propValue = updatePayload[i + 1];
setProp(domElement, tag, propKey, propValue, nextProps, null);
setProp(domElement, tag, propKey, propValue, nextProps, lastProps[propKey]);
}
}

Expand Down Expand Up @@ -2709,6 +2788,18 @@ function diffHydratedGenericElement(
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.
// So we remove them from the extra attributes warnings.
if (propKey === 'formAction') {
extraAttributes.delete('name');
extraAttributes.delete('formenctype');
extraAttributes.delete('formmethod');
extraAttributes.delete('formtarget');
} else {
extraAttributes.delete('enctype');
extraAttributes.delete('method');
extraAttributes.delete('target');
}
if (hasFormActionURL) {
// Expected
continue;
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom-bindings/src/client/ReactDOMInput.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ export function updateInput(
// Submit/reset inputs need the attribute removed completely to avoid
// blank-text buttons.
node.removeAttribute('value');
return;
}

if (disableInputAttributeSyncing) {
Expand Down
Loading

0 comments on commit 2fa6323

Please sign in to comment.