-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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.
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
Automatically reset forms after action finishes #28804
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,7 @@ import { | |
StoreConsistency, | ||
MountLayoutDev as MountLayoutDevEffect, | ||
MountPassiveDev as MountPassiveDevEffect, | ||
FormReset, | ||
} from './ReactFiberFlags'; | ||
import { | ||
HasEffect as HookHasEffect, | ||
|
@@ -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 { | ||
|
@@ -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; | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this actually have to be on the Fiber? Can't it just be scheduled on the Root with a list of forms to reset when the root commits? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That way you don't need all the special commit phase stuff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I'll need to think more about that, the scenario I had in mind was if the form is inside a Suspense boundary, the root could commit before the input's defaultValue had been updated. Usually this shouldn't happen because during a Transition we won't hide already visible forms, but during a popstate transition we might. The |
||
} | ||
|
||
function mountTransition(): [ | ||
boolean, | ||
(callback: () => void, options?: StartTransitionOptions) => void, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should start a campaign for javascript64