Skip to content
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

Scaffolding for requestFormReset API #28808

Merged
merged 1 commit into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';
import {validateLinkPropsForStyleResource} from '../shared/ReactDOMResourceValidation';
import escapeSelectorAttributeValueInsideDoubleQuotes from './escapeSelectorAttributeValueInsideDoubleQuotes';
import {flushSyncWork as flushSyncWorkOnAllRoots} from 'react-reconciler/src/ReactFiberWorkLoop';
import {requestFormReset as requestFormResetOnFiber} from 'react-reconciler/src/ReactFiberHooks';

import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals';

Expand Down Expand Up @@ -1928,6 +1929,7 @@ ReactDOMSharedInternals.d /* ReactDOMCurrentDispatcher */ = {
f /* flushSyncWork */: disableLegacyMode
? flushSyncWork
: previousDispatcher.f /* flushSyncWork */,
r: requestFormReset,
D /* prefetchDNS */: prefetchDNS,
C /* preconnect */: preconnect,
L /* preload */: preload,
Expand All @@ -1951,6 +1953,23 @@ function flushSyncWork() {
}
}

function requestFormReset(form: HTMLFormElement) {
const formInst = getInstanceFromNodeDOMTree(form);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should only forward the call if the instance isn't owned by this renderer? the float ones arguably only need to be dispatched once in a client dispatcher but that's because there isn't really any defined ownership to tie the request to a specific dispatcher but in this case only the renderer that created the form will be able to resolve the formInst right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll make it so the default implementation throws. The reconciler implementation will look for a matching instance, then call the previous one only if none exists. That way if there's no match, you get an error.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I guess that's not so bad with error minification. it's hard to understand how you would have gotten a form to pass to this before loading a reconciler so if this ever happens you are almost certainly passing in a dom node React didn't create

if (
formInst !== null &&
formInst.tag === HostComponent &&
formInst.type === 'form'
) {
requestFormResetOnFiber(formInst);
} else {
// This form was either not rendered by this React renderer (or it's an
// invalid type). Try the next one.
//
// The last implementation in the sequence will throw an error.
previousDispatcher.r(/* requestFormReset */ form);
}
}

// We expect this to get inlined. It is a function mostly to communicate the special nature of
// how we resolve the HoistableRoot for ReactDOM.pre*() methods. Because we support calling
// these methods outside of render there is no way to know which Document or ShadowRoot is 'scoped'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const previousDispatcher =
ReactDOMSharedInternals.d; /* ReactDOMCurrentDispatcher */
ReactDOMSharedInternals.d /* ReactDOMCurrentDispatcher */ = {
f /* flushSyncWork */: previousDispatcher.f /* flushSyncWork */,
r /* requestFormReset */: previousDispatcher.r /* requestFormReset */,
D /* prefetchDNS */: prefetchDNS,
C /* preconnect */: preconnect,
L /* preload */: preload,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ const previousDispatcher =
ReactDOMSharedInternals.d; /* ReactDOMCurrentDispatcher */
ReactDOMSharedInternals.d /* ReactDOMCurrentDispatcher */ = {
f /* flushSyncWork */: previousDispatcher.f /* flushSyncWork */,
r /* requestFormReset */: previousDispatcher.r /* requestFormReset */,
D /* prefetchDNS */: prefetchDNS,
C /* preconnect */: preconnect,
L /* preload */: preload,
Expand Down
6 changes: 6 additions & 0 deletions packages/react-dom-bindings/src/shared/ReactDOMFormActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {Awaited} from 'shared/ReactTypes';

import {enableAsyncActions} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import ReactDOMSharedInternals from 'shared/ReactDOMSharedInternals';

type FormStatusNotPending = {|
pending: false,
Expand Down Expand Up @@ -87,3 +88,8 @@ export function useFormState<S, P>(
return dispatcher.useFormState(action, initialState, permalink);
}
}

export function requestFormReset(form: HTMLFormElement) {
ReactDOMSharedInternals.d /* ReactDOMCurrentDispatcher */
.r(/* requestFormReset */ form);
}
1 change: 1 addition & 0 deletions packages/react-dom/index.classic.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export {
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
requestFormReset,
prefetchDNS,
preconnect,
preload,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/index.experimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export {
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
requestFormReset,
prefetchDNS,
preconnect,
preload,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
requestFormReset,
prefetchDNS,
preconnect,
preload,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/index.modern.fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export {
unstable_runWithPriority, // DO NOT USE: Temporarily exposed to migrate off of Scheduler.runWithPriority.
useFormStatus,
useFormState,
requestFormReset,
prefetchDNS,
preconnect,
preload,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/index.stable.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export {
unstable_batchedUpdates,
useFormStatus,
useFormState,
requestFormReset,
prefetchDNS,
preconnect,
preload,
Expand Down
8 changes: 8 additions & 0 deletions packages/react-dom/src/ReactDOMSharedInternals.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ export type ReactDOMInternalsDev = ReactDOMInternals & {

function noop() {}

function requestFormReset(element: HTMLFormElement) {
throw new Error(
'Invalid form element. requestFormReset must be passed a form that was ' +
'rendered by React.',
);
}

const DefaultDispatcher: HostDispatcher = {
f /* flushSyncWork */: noop,
r /* requestFormReset */: requestFormReset,
D /* prefetchDNS */: noop,
C /* preconnect */: noop,
L /* preload */: noop,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/ReactDOMSharedInternalsFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function noop() {}

const DefaultDispatcher: HostDispatcher = {
f /* flushSyncWork */: noop,
r /* requestFormReset */: noop,
D /* prefetchDNS */: noop,
C /* preconnect */: noop,
L /* preload */: noop,
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export {
export {
useFormStatus,
useFormState,
requestFormReset,
} from 'react-dom-bindings/src/shared/ReactDOMFormActions';

if (__DEV__) {
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOMFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export {
export {
useFormStatus,
useFormState,
requestFormReset,
} from 'react-dom-bindings/src/shared/ReactDOMFormActions';

if (__DEV__) {
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/shared/ReactDOMTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export type PreinitModuleScriptOptions = {

export type HostDispatcher = {
f /* flushSyncWork */: () => boolean | void,
r /* requestFormReset */: (form: HTMLFormElement) => void,
D /* prefetchDNS */: (href: string) => void,
C /* preconnect */: (href: string, crossOrigin?: ?CrossOriginEnum) => void,
L /* preload */: (
Expand Down
9 changes: 7 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -3008,13 +3008,18 @@ export function startHostTransition<F>(
// once more of this function is implemented.
() => {
// Automatically reset the form when the action completes.
requestFormReset(formFiber);
requestFormResetImpl(formFiber);
return callback(formData);
},
);
}

function requestFormReset(formFiber: Fiber) {
export function requestFormReset(formFiber: Fiber) {
// TODO: Not yet implemented. Need to upgrade the fiber to be stateful
// before scheduling the form reset.
}

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

if (__DEV__) {
Expand Down
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -506,5 +506,6 @@
"518": "Saw multiple hydration diff roots in a pass. This is a bug in React.",
"519": "Hydration Mismatch Exception: This is not a real error, and should not leak into userspace. If you're seeing this, it's likely a bug in React.",
"520": "There was an error during concurrent rendering but React was able to recover by instead synchronously rendering the entire root.",
"521": "flushSyncWork should not be called from builds that support legacy mode. This is a bug in React."
"521": "flushSyncWork should not be called from builds that support legacy mode. This is a bug in React.",
"522": "Invalid form element. requestFormReset must be passed a form that was rendered by React."
}