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

Cleanup enableFormActions flag #28614

Merged
merged 1 commit into from
Mar 25, 2024
Merged

Cleanup enableFormActions flag #28614

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
2 changes: 1 addition & 1 deletion packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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