Skip to content

Commit

Permalink
Allow empty string to be passed to formAction (#26379)
Browse files Browse the repository at this point in the history
We disallow empty strings for `href` and `src` since they're common
mistakes that end up loading the current page as a preload, image or
link. We also disallow it for `action`. You have to pass `null` which is
the same.

However, for `formAction` passing `null` is not the same as passing
empty string. Passing empty string overrides the form's action to be the
current page even if the form's action was set to something else.
There's no easy way to express the same thing `#` show up in the user
visible URLs and `?` clears the search params.

Since this is also not a common mistake, we can just allow this.
  • Loading branch information
sebmarkbage authored Mar 13, 2023
1 parent f828bad commit 2788d0d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 23 deletions.
14 changes: 13 additions & 1 deletion packages/react-dom-bindings/src/shared/DOMProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,19 @@ properties[xlinkHref] = new PropertyInfoRecord(
false, // removeEmptyString
);

['src', 'href', 'action', 'formAction'].forEach(attributeName => {
const formAction = 'formAction';
// $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
properties[formAction] = new PropertyInfoRecord(
'formAction',
STRING,
false, // mustUseProperty
'formaction', // attributeName
null, // attributeNamespace
true, // sanitizeURL
false, // removeEmptyString
);

['src', 'href', 'action'].forEach(attributeName => {
// $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
properties[attributeName] = new PropertyInfoRecord(
attributeName,
Expand Down
30 changes: 9 additions & 21 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,29 +533,17 @@ describe('ReactDOMComponent', () => {
expect(node.hasAttribute('action')).toBe(false);
});

it('should not add an empty formAction attribute', () => {
it('allows empty string of a formAction to override the default of a parent', () => {
const container = document.createElement('div');
expect(() =>
ReactDOM.render(<button formAction="" />, container),
).toErrorDev(
'An empty string ("") was passed to the formAction attribute. ' +
'To fix this, either do not render the element at all ' +
'or pass null to formAction instead of an empty string.',
);
const node = container.firstChild;
expect(node.hasAttribute('formAction')).toBe(false);

ReactDOM.render(<button formAction="abc" />, container);
expect(node.hasAttribute('formAction')).toBe(true);

expect(() =>
ReactDOM.render(<button formAction="" />, container),
).toErrorDev(
'An empty string ("") was passed to the formAction attribute. ' +
'To fix this, either do not render the element at all ' +
'or pass null to formAction instead of an empty string.',
ReactDOM.render(
<form action="hello">
<button formAction="" />,
</form>,
container,
);
expect(node.hasAttribute('formAction')).toBe(false);
const node = container.firstChild.firstChild;
expect(node.hasAttribute('formaction')).toBe(true);
expect(node.getAttribute('formaction')).toBe('');
});

it('should not filter attributes for custom elements', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export const disableInputAttributeSyncing = false;
// Filter certain DOM attributes (e.g. src, href) if their values are empty
// strings. This prevents e.g. <img src=""> from making an unnecessary HTTP
// request for certain browsers.
export const enableFilterEmptyStringAttributesDOM = false;
export const enableFilterEmptyStringAttributesDOM = __EXPERIMENTAL__;

// Changes the behavior for rendering custom elements in both server rendering
// and client rendering, mostly to allow JSX attributes to apply to the custom
Expand Down

0 comments on commit 2788d0d

Please sign in to comment.