Skip to content

Commit

Permalink
Automatically reset forms after action finishes (#28804)
Browse files Browse the repository at this point in the history
This updates the behavior of form actions to automatically reset the
form's uncontrolled inputs after the action finishes.

This is a frequent feature request for people using actions and it
aligns the behavior of client-side form submissions more closely with
MPA form submissions.

It has no impact on controlled form inputs. It's the same as if you
called `form.reset()` manually, except React handles the timing of when
the reset happens, which is tricky/impossible to get exactly right in
userspace.

The reset shouldn't happen until the UI has updated with the result of
the action. So, resetting inside the action is too early.

Resetting in `useEffect` is better, but it's later than ideal because
any effects that run before it will observe the state of the form before
it's been reset.

It needs to happen in the mutation phase of the transition. More
specifically, after all the DOM mutations caused by the transition have
been applied. That way the `defaultValue` of the inputs are updated
before the values are reset. The idea is that the `defaultValue`
represents the current, canonical value sent by the server.

Note: this change has no effect on form submissions that aren't
triggered by an action.
  • Loading branch information
acdlite authored and rickhanlonii committed Apr 11, 2024
1 parent deccced commit d9cc6e7
Show file tree
Hide file tree
Showing 11 changed files with 230 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/react-art/src/ReactFiberConfigART.js
Original file line number Diff line number Diff line change
Expand Up @@ -490,3 +490,4 @@ export function waitForCommitToBeReady() {
}

export const NotPendingTransition = null;
export function resetFormInstance() {}
5 changes: 5 additions & 0 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -3441,3 +3441,8 @@ function insertStylesheetIntoRoot(
}

export const NotPendingTransition: TransitionStatus = NotPending;

export type FormInstance = HTMLFormElement;
export function resetFormInstance(form: FormInstance): void {
form.reset();
}
80 changes: 80 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('ReactDOMForm', () => {
let useState;
let Suspense;
let startTransition;
let use;
let textCache;
let useFormStatus;
let useActionState;
Expand All @@ -55,6 +56,7 @@ describe('ReactDOMForm', () => {
useState = React.useState;
Suspense = React.Suspense;
startTransition = React.startTransition;
use = React.use;
useFormStatus = ReactDOM.useFormStatus;
container = document.createElement('div');
document.body.appendChild(container);
Expand Down Expand Up @@ -1334,4 +1336,82 @@ describe('ReactDOMForm', () => {
assertLog(['1']);
expect(container.textContent).toBe('1');
});

test('uncontrolled form inputs are reset after the action completes', async () => {
const formRef = React.createRef();
const inputRef = React.createRef();
const divRef = React.createRef();

function App({promiseForUsername}) {
// Make this suspensey to simulate RSC streaming.
const username = use(promiseForUsername);

return (
<form
ref={formRef}
action={async formData => {
const rawUsername = formData.get('username');
const normalizedUsername = rawUsername.trim().toLowerCase();

Scheduler.log(`Async action started`);
await getText('Wait');

// Update the app with new data. This is analagous to re-rendering
// from the root with a new RSC payload.
startTransition(() => {
root.render(
<App promiseForUsername={getText(normalizedUsername)} />,
);
});
}}>
<input
ref={inputRef}
text="text"
name="username"
defaultValue={username}
/>
<div ref={divRef}>
<Text text={'Current username: ' + username} />
</div>
</form>
);
}

// Initial render
const root = ReactDOMClient.createRoot(container);
const promiseForInitialUsername = getText('(empty)');
await resolveText('(empty)');
await act(() =>
root.render(<App promiseForUsername={promiseForInitialUsername} />),
);
assertLog(['Current username: (empty)']);
expect(divRef.current.textContent).toEqual('Current username: (empty)');

// Dirty the uncontrolled input
inputRef.current.value = ' AcdLite ';

// Submit the form. This will trigger an async action.
await submit(formRef.current);
assertLog(['Async action started']);
expect(inputRef.current.value).toBe(' AcdLite ');

// Finish the async action. This will trigger a re-render from the root with
// new data from the "server", which suspends.
//
// The form should not reset yet because we need to update `defaultValue`
// first. So we wait for the render to complete.
await act(() => resolveText('Wait'));
assertLog([]);
// The DOM input is still dirty.
expect(inputRef.current.value).toBe(' AcdLite ');
// The React tree is suspended.
expect(divRef.current.textContent).toEqual('Current username: (empty)');

// Unsuspend and finish rendering. Now the form should be reset.
await act(() => resolveText('acdlite'));
assertLog(['Current username: acdlite']);
// The form was reset to the new value from the server.
expect(inputRef.current.value).toBe('acdlite');
expect(divRef.current.textContent).toEqual('Current username: acdlite');
});
});
3 changes: 3 additions & 0 deletions packages/react-native-renderer/src/ReactFiberConfigFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,9 @@ export function waitForCommitToBeReady(): null {

export const NotPendingTransition: TransitionStatus = null;

export type FormInstance = Instance;
export function resetFormInstance(form: Instance): void {}

// -------------------
// Microtasks
// -------------------
Expand Down
3 changes: 3 additions & 0 deletions packages/react-native-renderer/src/ReactFiberConfigNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -549,3 +549,6 @@ export function waitForCommitToBeReady(): null {
}

export const NotPendingTransition: TransitionStatus = null;

export type FormInstance = Instance;
export function resetFormInstance(form: Instance): void {}
4 changes: 4 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ type SuspenseyCommitSubscription = {

export type TransitionStatus = mixed;

export type FormInstance = Instance;

const NO_CONTEXT = {};
const UPPERCASE_CONTEXT = {};
if (__DEV__) {
Expand Down Expand Up @@ -632,6 +634,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
waitForCommitToBeReady,

NotPendingTransition: (null: TransitionStatus),

resetFormInstance(form: Instance) {},
};

const hostConfig = useMutation
Expand Down
53 changes: 53 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
ChildSet,
UpdatePayload,
HoistableRoot,
FormInstance,
} from './ReactFiberConfig';
import type {Fiber, FiberRoot} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane';
Expand Down Expand Up @@ -97,6 +98,7 @@ import {
Visibility,
ShouldSuspendCommit,
MaySuspendCommit,
FormReset,
} from './ReactFiberFlags';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import {
Expand Down Expand Up @@ -163,6 +165,7 @@ import {
prepareToCommitHoistables,
suspendInstance,
suspendResource,
resetFormInstance,
} from './ReactFiberConfig';
import {
captureCommitPhaseError,
Expand Down Expand Up @@ -226,6 +229,9 @@ if (__DEV__) {
let offscreenSubtreeIsHidden: boolean = false;
let offscreenSubtreeWasHidden: boolean = false;

// Used to track if a form needs to be reset at the end of the mutation phase.
let needsFormReset = false;

const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set;

let nextEffect: Fiber | null = null;
Expand Down Expand Up @@ -2776,6 +2782,20 @@ function commitMutationEffectsOnFiber(
}
}
}

if (flags & FormReset) {
needsFormReset = true;
if (__DEV__) {
if (finishedWork.type !== 'form') {
// Paranoid coding. In case we accidentally start using the
// FormReset bit for something else.
console.error(
'Unexpected host component type. Expected a form. This is a ' +
'bug in React.',
);
}
}
}
}
return;
}
Expand Down Expand Up @@ -2852,6 +2872,21 @@ function commitMutationEffectsOnFiber(
}
}
}

if (needsFormReset) {
// A form component requested to be reset during this commit. We do this
// after all mutations in the rest of the tree so that `defaultValue`
// will already be updated. This way you can update `defaultValue` using
// data sent by the server as a result of the form submission.
//
// Theoretically we could check finishedWork.subtreeFlags & FormReset,
// but the FormReset bit is overloaded with other flags used by other
// fiber types. So this extra variable lets us skip traversing the tree
// except when a form was actually submitted.
needsFormReset = false;
recursivelyResetForms(finishedWork);
}

return;
}
case HostPortal: {
Expand Down Expand Up @@ -3091,6 +3126,24 @@ function commitReconciliationEffects(finishedWork: Fiber) {
}
}

function recursivelyResetForms(parentFiber: Fiber) {
if (parentFiber.subtreeFlags & FormReset) {
let child = parentFiber.child;
while (child !== null) {
resetFormOnFiber(child);
child = child.sibling;
}
}
}

function resetFormOnFiber(fiber: Fiber) {
recursivelyResetForms(fiber);
if (fiber.tag === HostComponent && fiber.flags & FormReset) {
const formInstance: FormInstance = fiber.stateNode;
resetFormInstance(formInstance);
}
}

export function commitLayoutEffects(
finishedWork: Fiber,
root: FiberRoot,
Expand Down
4 changes: 3 additions & 1 deletion packages/react-reconciler/src/ReactFiberFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const StoreConsistency = /* */ 0b0000000000000100000000000000
export const ScheduleRetry = StoreConsistency;
export const ShouldSuspendCommit = Visibility;
export const DidDefer = ContentReset;
export const FormReset = Snapshot;

export const LifecycleEffectMask =
Passive | Update | Callback | Ref | Snapshot | StoreConsistency;
Expand Down Expand Up @@ -95,7 +96,8 @@ export const MutationMask =
ContentReset |
Ref |
Hydrating |
Visibility;
Visibility |
FormReset;
export const LayoutMask = Update | Callback | Ref | Visibility;

// TODO: Split into PassiveMountMask and PassiveUnmountMask
Expand Down
77 changes: 73 additions & 4 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import {
StoreConsistency,
MountLayoutDev as MountLayoutDevEffect,
MountPassiveDev as MountPassiveDevEffect,
FormReset,
} from './ReactFiberFlags';
import {
HasEffect as HookHasEffect,
Expand Down Expand Up @@ -844,15 +845,29 @@ export function TransitionAwareHostComponent(): TransitionStatus {
if (!enableAsyncActions) {
throw new Error('Not implemented.');
}

const dispatcher: any = ReactSharedInternals.H;
const [maybeThenable] = dispatcher.useState();
let nextState;
if (typeof maybeThenable.then === 'function') {
const thenable: Thenable<TransitionStatus> = (maybeThenable: any);
return useThenable(thenable);
nextState = useThenable(thenable);
} else {
const status: TransitionStatus = maybeThenable;
return status;
nextState = status;
}

// The "reset state" is an object. If it changes, that means something
// requested that we reset the form.
const [nextResetState] = dispatcher.useState();
const prevResetState =
currentHook !== null ? currentHook.memoizedState : null;
if (prevResetState !== nextResetState) {
// Schedule a form reset
currentlyRenderingFiber.flags |= FormReset;
}

return nextState;
}

export function checkDidRenderIdHook(): boolean {
Expand Down Expand Up @@ -2948,7 +2963,30 @@ export function startHostTransition<F>(
next: null,
};

// Add the state hook to both fiber alternates. The idea is that the fiber
// We use another state hook to track whether the form needs to be reset.
// The state is an empty object. To trigger a reset, we update the state
// to a new object. Then during rendering, we detect that the state has
// changed and schedule a commit effect.
const initialResetState = {};
const newResetStateQueue: UpdateQueue<Object, Object> = {
pending: null,
lanes: NoLanes,
// We're going to cheat and intentionally not create a bound dispatch
// method, because we can call it directly in startTransition.
dispatch: (null: any),
lastRenderedReducer: basicStateReducer,
lastRenderedState: initialResetState,
};
const resetStateHook: Hook = {
memoizedState: initialResetState,
baseState: initialResetState,
baseQueue: null,
queue: newResetStateQueue,
next: null,
};
stateHook.next = resetStateHook;

// Add the hook list to both fiber alternates. The idea is that the fiber
// had this hook all along.
formFiber.memoizedState = stateHook;
const alternate = formFiber.alternate;
Expand All @@ -2968,10 +3006,41 @@ export function startHostTransition<F>(
NoPendingHostTransition,
// TODO: We can avoid this extra wrapper, somehow. Figure out layering
// once more of this function is implemented.
() => callback(formData),
() => {
// Automatically reset the form when the action completes.
requestFormReset(formFiber);
return callback(formData);
},
);
}

function requestFormReset(formFiber: Fiber) {
const transition = requestCurrentTransition();

if (__DEV__) {
if (transition === null) {
// An optimistic update occurred, but startTransition is not on the stack.
// The form reset will be scheduled at default (sync) priority, which
// is probably not what the user intended. Most likely because the
// requestFormReset call happened after an `await`.
// TODO: Theoretically, requestFormReset is still useful even for
// non-transition updates because it allows you to update defaultValue
// synchronously and then wait to reset until after the update commits.
// I've chosen to warn anyway because it's more likely the `await` mistake
// described above. But arguably we shouldn't.
console.error(
'requestFormReset was called outside a transition or action. To ' +
'fix, move to an action, or wrap with startTransition.',
);
}
}

const newResetState = {};
const resetStateHook: Hook = (formFiber.memoizedState.next: any);
const resetStateQueue = resetStateHook.queue;
dispatchSetState(formFiber, resetStateQueue, newResetState);
}

function mountTransition(): [
boolean,
(callback: () => void, options?: StartTransitionOptions) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export opaque type TimeoutHandle = mixed; // eslint-disable-line no-undef
export opaque type NoTimeout = mixed; // eslint-disable-line no-undef
export opaque type RendererInspectionConfig = mixed; // eslint-disable-line no-undef
export opaque type TransitionStatus = mixed; // eslint-disable-line no-undef
export opaque type FormInstance = mixed; // eslint-disable-line no-undef
export type EventResponder = any;

export const getPublicInstance = $$$config.getPublicInstance;
Expand Down Expand Up @@ -78,6 +79,7 @@ export const startSuspendingCommit = $$$config.startSuspendingCommit;
export const suspendInstance = $$$config.suspendInstance;
export const waitForCommitToBeReady = $$$config.waitForCommitToBeReady;
export const NotPendingTransition = $$$config.NotPendingTransition;
export const resetFormInstance = $$$config.resetFormInstance;

// -------------------
// Microtasks
Expand Down
Loading

0 comments on commit d9cc6e7

Please sign in to comment.