Skip to content

Commit

Permalink
Fix: Coerce form submitter action prop
Browse files Browse the repository at this point in the history
When handling form action functions, we should coerce the submitter
formAction prop using the same logic as ReactDOMComponent before
deciding whether it should override the action on the <form>.

Before this fix, if the submitter's `formAction` prop was a symbol or
a boolean, it would block the form's action function from firing, but
the correct behavior is that symbols and booleans should be treated as
if they were null/empty, because that's how we treat those values when
setting DOM properties.
  • Loading branch information
acdlite committed May 9, 2024
1 parent 04b0588 commit 7612ce3
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,36 @@ import type {EventSystemFlags} from '../EventSystemFlags';
import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {FormStatus} from 'react-dom-bindings/src/shared/ReactDOMFormActions';

import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';
import {getFiberCurrentPropsFromNode} from '../../client/ReactDOMComponentTree';
import {startHostTransition} from 'react-reconciler/src/ReactFiberReconciler';
import sanitizeURL from 'react-dom-bindings/src/shared/sanitizeURL';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';

import {SyntheticEvent} from '../SyntheticEvent';

function coerceFormActionProp(
actionProp: mixed,
): string | (FormData => void | Promise<void>) | null {
// This should match the logic in ReactDOMComponent
if (
actionProp == null ||
typeof actionProp === 'symbol' ||
typeof actionProp === 'boolean'
) {
return null;
} else if (typeof actionProp === 'function') {
return (actionProp: any);
} else {
if (__DEV__) {
checkAttributeStringCoercion(actionProp, 'action');
}
return (sanitizeURL(
enableTrustedTypesIntegration ? actionProp : '' + (actionProp: any),
): any);
}
}

/**
* This plugin invokes action functions on forms, inputs and buttons if
* the form doesn't prevent default.
Expand Down Expand Up @@ -48,10 +73,12 @@ function extractEvents(
let submitterAction;
if (submitter) {
const submitterProps = getFiberCurrentPropsFromNode(submitter);
submitterAction = submitterProps
? (submitterProps: any).formAction
: submitter.getAttribute('formAction');
if (submitterAction != null) {
submitterAction = coerceFormActionProp(
submitterProps
? (submitterProps: any).formAction
: submitter.getAttribute('formAction'),
);
if (submitterAction !== null) {
// The submitter overrides the form action.
action = submitterAction;
// If the action is a function, we don't want to pass its name
Expand Down
47 changes: 47 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('ReactDOMForm', () => {
let ReactDOMClient;
let Scheduler;
let assertLog;
let assertConsoleErrorDev;
let waitForThrow;
let useState;
let Suspense;
Expand All @@ -54,6 +55,8 @@ describe('ReactDOMForm', () => {
act = require('internal-test-utils').act;
assertLog = require('internal-test-utils').assertLog;
waitForThrow = require('internal-test-utils').waitForThrow;
assertConsoleErrorDev =
require('internal-test-utils').assertConsoleErrorDev;
useState = React.useState;
Suspense = React.Suspense;
startTransition = React.startTransition;
Expand Down Expand Up @@ -1782,4 +1785,48 @@ describe('ReactDOMForm', () => {
// The form was reset even though the action didn't finish.
expect(inputRef.current.value).toBe('Initial');
});

test("regression: submitter's formAction prop is coerced correctly before checking if it exists", async () => {
function App({submitterAction}) {
return (
<form action={() => Scheduler.log('Form action')}>
<button ref={buttonRef} type="submit" formAction={submitterAction} />
</form>
);
}

const buttonRef = React.createRef();
const root = ReactDOMClient.createRoot(container);

await act(() =>
root.render(
<App submitterAction={() => Scheduler.log('Button action')} />,
),
);
await submit(buttonRef.current);
assertLog(['Button action']);

// When there's no button action, the form action should fire
await act(() => root.render(<App submitterAction={null} />));
await submit(buttonRef.current);
assertLog(['Form action']);

// Symbols are coerced to null, so this should fire the form action
await act(() => root.render(<App submitterAction={Symbol()} />));
assertConsoleErrorDev(['Invalid value for prop `formAction`']);
await submit(buttonRef.current);
assertLog(['Form action']);

// Booleans are coerced to null, so this should fire the form action
await act(() => root.render(<App submitterAction={true} />));
await submit(buttonRef.current);
assertLog(['Form action']);

// A string on the submitter should prevent the form action from firing
// and trigger the native behavior
await act(() => root.render(<App submitterAction="https://react.dev/" />));
await expect(submit(buttonRef.current)).rejects.toThrow(
'Navigate to: https://react.dev/',
);
});
});

0 comments on commit 7612ce3

Please sign in to comment.