Skip to content

Commit

Permalink
Cleanup enableFormActions flag (#28614)
Browse files Browse the repository at this point in the history 
Cleanup enableFormActions flag
  • Loading branch information
@kassens
kassens committed Mar 25, 2024
1 parent e373190 commit 527ed72
Show file tree
Hide file tree
Showing 30 changed files with 184 additions and 310 deletions.
2 changes: 1 addition & 1 deletion packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2672,7 +2672,7 @@ describe('ReactHooksInspectionIntegration', () => {
`);
});

// @gate enableFormActions && enableAsyncActions
// @gate enableAsyncActions
it('should support useFormState hook', async () => {
function Foo() {
const [value] = ReactDOM.useFormState(function increment(n) {
Expand Down
159 changes: 70 additions & 89 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ import {
enableBigIntSupport,
enableCustomElementPropertySupport,
enableClientRenderFallbackOnTextMismatch,
enableFormActions,
disableIEWorkarounds,
enableTrustedTypesIntegration,
enableFilterEmptyStringAttributesDOM,
Expand Down Expand Up @@ -498,71 +497,54 @@ function setProp(
if (__DEV__) {
validateFormActionInDevelopment(tag, key, value, props);
}
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 (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') ||
typeof value === 'symbol' ||
typeof value === 'boolean'
) {
Expand Down Expand Up @@ -2435,35 +2417,33 @@ function diffHydratedGenericElement(
);
continue;
case 'action':
case 'formAction':
if (enableFormActions) {
const serverValue = domElement.getAttribute(propKey);
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');
}
// 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 (serverValue === EXPECTED_FORM_ACTION_URL) {
extraAttributes.delete(propKey.toLowerCase());
warnForPropDifference(propKey, 'function', value);
continue;
case 'formAction': {
const serverValue = domElement.getAttribute(propKey);
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');
}
// 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 (serverValue === EXPECTED_FORM_ACTION_URL) {
extraAttributes.delete(propKey.toLowerCase());
warnForPropDifference(propKey, 'function', value);
continue;
}
hydrateSanitizedAttribute(
domElement,
Expand All @@ -2473,6 +2453,7 @@ function diffHydratedGenericElement(
extraAttributes,
);
continue;
}
case 'xlinkHref':
hydrateSanitizedAttribute(
domElement,
Expand Down
21 changes: 4 additions & 17 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ import {
enableCreateEventHandleAPI,
enableScopeAPI,
enableTrustedTypesIntegration,
enableFormActions,
enableAsyncActions,
} from 'shared/ReactFeatureFlags';
import {
Expand Down Expand Up @@ -1040,11 +1039,7 @@ export function canHydrateInstance(
if (element.nodeName.toLowerCase() !== type.toLowerCase()) {
if (!inRootOrSingleton) {
// Usually we error for mismatched tags.
if (
enableFormActions &&
element.nodeName === 'INPUT' &&
(element: any).type === 'hidden'
) {
if (element.nodeName === 'INPUT' && (element: any).type === 'hidden') {
// If we have extra hidden inputs, we don't mismatch. This allows us to embed
// extra form data in the original form.
} else {
Expand All @@ -1054,11 +1049,7 @@ export function canHydrateInstance(
// In root or singleton parents we skip past mismatched instances.
} else if (!inRootOrSingleton) {
// Match
if (
enableFormActions &&
type === 'input' &&
(element: any).type === 'hidden'
) {
if (type === 'input' && (element: any).type === 'hidden') {
if (__DEV__) {
checkAttributeStringCoercion(anyProps.name, 'name');
}
Expand Down Expand Up @@ -1190,7 +1181,6 @@ export function canHydrateTextInstance(

while (instance.nodeType !== TEXT_NODE) {
if (
enableFormActions &&
instance.nodeType === ELEMENT_NODE &&
instance.nodeName === 'INPUT' &&
(instance: any).type === 'hidden'
Expand Down Expand Up @@ -1316,8 +1306,7 @@ function getNextHydratable(node: ?Node) {
nodeData === SUSPENSE_START_DATA ||
nodeData === SUSPENSE_FALLBACK_START_DATA ||
nodeData === SUSPENSE_PENDING_START_DATA ||
(enableFormActions &&
enableAsyncActions &&
(enableAsyncActions &&
(nodeData === FORM_STATE_IS_MATCHING ||
nodeData === FORM_STATE_IS_NOT_MATCHING))
) {
Expand Down Expand Up @@ -1512,9 +1501,7 @@ export function commitHydratedSuspenseInstance(
export function shouldDeleteUnhydratedTailInstances(
parentType: string,
): boolean {
return (
!enableFormActions || (parentType !== 'form' && parentType !== 'button')
);
return parentType !== 'form' && parentType !== 'button';
}

export function didNotMatchHydratedContainerTextInstance(
Expand Down
21 changes: 9 additions & 12 deletions packages/react-dom-bindings/src/events/DOMPluginEventSystem.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import {
enableLegacyFBSupport,
enableCreateEventHandleAPI,
enableScopeAPI,
enableFormActions,
} from 'shared/ReactFeatureFlags';
import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener';
import {
Expand Down Expand Up @@ -169,17 +168,15 @@ function extractEvents(
eventSystemFlags,
targetContainer,
);
if (enableFormActions) {
FormActionEventPlugin.extractEvents(
dispatchQueue,
domEventName,
targetInst,
nativeEvent,
nativeEventTarget,
eventSystemFlags,
targetContainer,
);
}
FormActionEventPlugin.extractEvents(
dispatchQueue,
domEventName,
targetInst,
nativeEvent,
nativeEventTarget,
eventSystemFlags,
targetContainer,
);
}
}

Expand Down
Loading

0 comments on commit 527ed72

Please sign in to comment.