Skip to content

Commit e8e533c

Browse files
sebmarkbagesebmarkbage
committed
Restore server controlled form fields to whatever they should be
Fizz can emit whatever for the SSR version of these fields when it's a function actual 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.
1 parent 36e4cbe commit e8e533c

File tree

3 files changed

+239
-19
lines changed

3 files changed

+239
-19
lines changed

‎packages/react-dom-bindings/src/client/ReactDOMComponent.js

+74-18
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,68 @@ function setProp(
483483
case 'action':
484484
case 'formAction': {
485485
// TODO: Consider moving these special cases to the form, input and button tags.
486+
if (enableFormActions) {
487+
if (typeof value === 'function') {
488+
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
489+
// because we'll preventDefault, but it can happen if a form is manually submitted or
490+
// if someone calls stopPropagation before React gets the event.
491+
// If CSP is used to block javascript: URLs that's fine too. It just won't show this
492+
// error message but the URL will be logged.
493+
domElement.setAttribute(
494+
key,
495+
// eslint-disable-next-line no-script-url
496+
"javascript:throw new Error('" +
497+
'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
498+
"consider using form.requestSubmit() instead. If you're trying to use " +
499+
'event.stopPropagation() in a submit event handler, consider also calling ' +
500+
'event.preventDefault().' +
501+
"')",
502+
);
503+
break;
504+
} else if (typeof prevValue === 'function') {
505+
// When we're switching off a Server Action that was originally hydrated.
506+
// The server control these fields during SSR that are now trailing.
507+
// The regular diffing doesn't apply since we compare against the previous props.
508+
// Instead, we need to force them to be set to whatever they should be now.
509+
// This would be a lot cleaner if we did this whole fork in the per-tag approach.
510+
if (key === 'formAction') {
511+
if (tag !== 'input') {
512+
// Setting the name here isn't completely safe for inputs if this is switching
513+
// to become a radio button. In that case we let the tag based override take
514+
// control.
515+
setProp(domElement, tag, 'name', props.name, props, null);
516+
}
517+
setProp(
518+
domElement,
519+
tag,
520+
'formEncType',
521+
props.formEncType,
522+
props,
523+
null,
524+
);
525+
setProp(
526+
domElement,
527+
tag,
528+
'formMethod',
529+
props.formMethod,
530+
props,
531+
null,
532+
);
533+
setProp(
534+
domElement,
535+
tag,
536+
'formTarget',
537+
props.formTarget,
538+
props,
539+
null,
540+
);
541+
} else {
542+
setProp(domElement, tag, 'encType', props.encType, props, null);
543+
setProp(domElement, tag, 'method', props.method, props, null);
544+
setProp(domElement, tag, 'target', props.target, props, null);
545+
}
546+
}
547+
}
486548
if (
487549
value == null ||
488550
(!enableFormActions && typeof value === 'function') ||
@@ -495,24 +557,6 @@ function setProp(
495557
if (__DEV__) {
496558
validateFormActionInDevelopment(tag, key, value, props);
497559
}
498-
if (enableFormActions && typeof value === 'function') {
499-
// Set a javascript URL that doesn't do anything. We don't expect this to be invoked
500-
// because we'll preventDefault, but it can happen if a form is manually submitted or
501-
// if someone calls stopPropagation before React gets the event.
502-
// If CSP is used to block javascript: URLs that's fine too. It just won't show this
503-
// error message but the URL will be logged.
504-
domElement.setAttribute(
505-
key,
506-
// eslint-disable-next-line no-script-url
507-
"javascript:throw new Error('" +
508-
'A React form was unexpectedly submitted. If you called form.submit() manually, ' +
509-
"consider using form.requestSubmit() instead. If you're trying to use " +
510-
'event.stopPropagation() in a submit event handler, consider also calling ' +
511-
'event.preventDefault().' +
512-
"')",
513-
);
514-
break;
515-
}
516560
// `setAttribute` with objects becomes only `[object]` in IE8/9,
517561
// ('' + value) makes it output the correct toString()-value.
518562
if (__DEV__) {
@@ -2709,6 +2753,18 @@ function diffHydratedGenericElement(
27092753
const hasFormActionURL = serverValue === EXPECTED_FORM_ACTION_URL;
27102754
if (typeof value === 'function') {
27112755
extraAttributes.delete(propKey.toLowerCase());
2756+
// The server can set these extra properties to implement actions.
2757+
// So we remove them from the extra attributes warnings.
2758+
if (propKey === 'formAction') {
2759+
extraAttributes.delete('name');
2760+
extraAttributes.delete('formenctype');
2761+
extraAttributes.delete('formmethod');
2762+
extraAttributes.delete('formtarget');
2763+
} else {
2764+
extraAttributes.delete('enctype');
2765+
extraAttributes.delete('method');
2766+
extraAttributes.delete('target');
2767+
}
27122768
if (hasFormActionURL) {
27132769
// Expected
27142770
continue;

‎packages/react-dom-bindings/src/client/ReactDOMInput.js

-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ export function updateInput(
131131
// Submit/reset inputs need the attribute removed completely to avoid
132132
// blank-text buttons.
133133
node.removeAttribute('value');
134-
return;
135134
}
136135

137136
if (disableInputAttributeSyncing) {

‎packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js

+165
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,169 @@ describe('ReactDOMFizzForm', () => {
195195
'Prop `action` did not match. Server: "action" Client: "function action(formData) {}"',
196196
);
197197
});
198+
199+
// @gate enableFormActions
200+
it('should reset form fields after you update away from hydrated function', async () => {
201+
const formRef = React.createRef();
202+
const inputRef = React.createRef();
203+
const buttonRef = React.createRef();
204+
function action(formData) {}
205+
function App({isUpdate}) {
206+
return (
207+
<form
208+
action={isUpdate ? 'action' : action}
209+
ref={formRef}
210+
method={isUpdate ? 'POST' : null}>
211+
<input
212+
type="submit"
213+
formAction={isUpdate ? 'action' : action}
214+
ref={inputRef}
215+
formTarget={isUpdate ? 'elsewhere' : null}
216+
/>
217+
<button
218+
formAction={isUpdate ? 'action' : action}
219+
ref={buttonRef}
220+
formEncType={isUpdate ? 'multipart/form-data' : null}
221+
/>
222+
</form>
223+
);
224+
}
225+
226+
const stream = await ReactDOMServer.renderToReadableStream(<App />);
227+
await readIntoContainer(stream);
228+
let root;
229+
await act(async () => {
230+
root = ReactDOMClient.hydrateRoot(container, <App />);
231+
});
232+
await act(async () => {
233+
root.render(<App isUpdate={true} />);
234+
});
235+
expect(formRef.current.getAttribute('action')).toBe('action');
236+
expect(formRef.current.hasAttribute('encType')).toBe(false);
237+
expect(formRef.current.getAttribute('method')).toBe('POST');
238+
expect(formRef.current.hasAttribute('target')).toBe(false);
239+
240+
expect(inputRef.current.getAttribute('formAction')).toBe('action');
241+
expect(inputRef.current.hasAttribute('name')).toBe(false);
242+
expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
243+
expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
244+
expect(inputRef.current.getAttribute('formTarget')).toBe('elsewhere');
245+
246+
expect(buttonRef.current.getAttribute('formAction')).toBe('action');
247+
expect(buttonRef.current.hasAttribute('name')).toBe(false);
248+
expect(buttonRef.current.getAttribute('formEncType')).toBe(
249+
'multipart/form-data',
250+
);
251+
expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
252+
expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
253+
});
254+
255+
// @gate enableFormActions
256+
it('should reset form fields after you remove a hydrated function', async () => {
257+
const formRef = React.createRef();
258+
const inputRef = React.createRef();
259+
const buttonRef = React.createRef();
260+
function action(formData) {}
261+
function App({isUpdate}) {
262+
return (
263+
<form action={isUpdate ? undefined : action} ref={formRef}>
264+
<input
265+
type="submit"
266+
formAction={isUpdate ? undefined : action}
267+
ref={inputRef}
268+
/>
269+
<button formAction={isUpdate ? undefined : action} ref={buttonRef} />
270+
</form>
271+
);
272+
}
273+
274+
const stream = await ReactDOMServer.renderToReadableStream(<App />);
275+
await readIntoContainer(stream);
276+
let root;
277+
await act(async () => {
278+
root = ReactDOMClient.hydrateRoot(container, <App />);
279+
});
280+
await act(async () => {
281+
root.render(<App isUpdate={true} />);
282+
});
283+
expect(formRef.current.hasAttribute('action')).toBe(false);
284+
expect(formRef.current.hasAttribute('encType')).toBe(false);
285+
expect(formRef.current.hasAttribute('method')).toBe(false);
286+
expect(formRef.current.hasAttribute('target')).toBe(false);
287+
288+
expect(inputRef.current.hasAttribute('formAction')).toBe(false);
289+
expect(inputRef.current.hasAttribute('name')).toBe(false);
290+
expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
291+
expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
292+
expect(inputRef.current.hasAttribute('formTarget')).toBe(false);
293+
294+
expect(buttonRef.current.hasAttribute('formAction')).toBe(false);
295+
expect(buttonRef.current.hasAttribute('name')).toBe(false);
296+
expect(buttonRef.current.hasAttribute('formEncType')).toBe(false);
297+
expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
298+
expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
299+
});
300+
301+
// @gate enableFormActions
302+
it('should restore the form fields even if they were incorrectly set', async () => {
303+
const formRef = React.createRef();
304+
const inputRef = React.createRef();
305+
const buttonRef = React.createRef();
306+
function action(formData) {}
307+
function App({isUpdate}) {
308+
return (
309+
<form
310+
action={isUpdate ? 'action' : action}
311+
ref={formRef}
312+
method="DELETE">
313+
<input
314+
type="submit"
315+
formAction={isUpdate ? 'action' : action}
316+
ref={inputRef}
317+
formTarget="elsewhere"
318+
/>
319+
<button
320+
formAction={isUpdate ? 'action' : action}
321+
ref={buttonRef}
322+
formEncType="text/plain"
323+
/>
324+
</form>
325+
);
326+
}
327+
328+
// Specifying the extra form fields are a DEV error, but we expect it
329+
// to eventually still be patched up after an update.
330+
await expect(async () => {
331+
const stream = await ReactDOMServer.renderToReadableStream(<App />);
332+
await readIntoContainer(stream);
333+
}).toErrorDev([
334+
'Cannot specify a encType or method for a form that specifies a function as the action.',
335+
'Cannot specify a formTarget for a button that specifies a function as a formAction.',
336+
]);
337+
let root;
338+
await expect(async () => {
339+
await act(async () => {
340+
root = ReactDOMClient.hydrateRoot(container, <App />);
341+
});
342+
}).toErrorDev(['Prop `formTarget` did not match.']);
343+
await act(async () => {
344+
root.render(<App isUpdate={true} />);
345+
});
346+
expect(formRef.current.getAttribute('action')).toBe('action');
347+
expect(formRef.current.hasAttribute('encType')).toBe(false);
348+
expect(formRef.current.getAttribute('method')).toBe('DELETE');
349+
expect(formRef.current.hasAttribute('target')).toBe(false);
350+
351+
expect(inputRef.current.getAttribute('formAction')).toBe('action');
352+
expect(inputRef.current.hasAttribute('name')).toBe(false);
353+
expect(inputRef.current.hasAttribute('formEncType')).toBe(false);
354+
expect(inputRef.current.hasAttribute('formMethod')).toBe(false);
355+
expect(inputRef.current.getAttribute('formTarget')).toBe('elsewhere');
356+
357+
expect(buttonRef.current.getAttribute('formAction')).toBe('action');
358+
expect(buttonRef.current.hasAttribute('name')).toBe(false);
359+
expect(buttonRef.current.getAttribute('formEncType')).toBe('text/plain');
360+
expect(buttonRef.current.hasAttribute('formMethod')).toBe(false);
361+
expect(buttonRef.current.hasAttribute('formTarget')).toBe(false);
362+
});
198363
});

0 commit comments

Comments
 (0)