-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
feat(react-router): add useSubmit/useFetcher to createMemoryRouter
#10390
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
Conversation
🦋 Changeset detectedLatest commit: b0a66bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| return navigate; | ||
| } | ||
|
|
||
| type SubmitPayload = NonNullable<unknown> | null; |
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.
Can only submit raw values via createMemoryRouter, no submitting from DOM elements, FormData, or URLSearchParams
| // Base options shared between fetch() and navigate() | ||
| let opts = { | ||
| payload, | ||
| formMethod: options.method || "get", | ||
| formEncType: options.encType, | ||
| action: routerAction, | ||
| }; |
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.
No getFormSubmissionInfo here - just pass along the payload directly
| export type FetcherWithMethods<TData> = Fetcher<TData> & { | ||
| submit: ( | ||
| target: SubmitPayload, | ||
| // Fetchers cannot replace/preventScrollReset because they are not | ||
| // navigation events | ||
| options?: Omit<SubmitOptions, "replace" | "preventScrollReset"> | ||
| ) => void; | ||
| load: (href: string | LoaderFunction) => 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.
Not FetcherWithComponents because there's no <Form> in react-router
| return useSubmitImpl(); | ||
| } | ||
|
|
||
| function useSubmitImpl( |
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.
There's room for some DRY here if we want to - but for the first pass I chose to just add this hook here and change in place. eventually we could do something like useSubmitImpl(getRouterOpts, fetcherKey, fetcherRouteId) where getRouterOpts would be the DOM-aware way to gather a submission and the memory version would pass a simple non-DOM-aware implementation.
But it makes the types tricky since the returned SubmitFunction function types are then dependent on the getRouterOpts implementation, so it doesn't feel like that would be quite the right abstraction.
| ...(pendingActionData ? { actionData: pendingActionData } : {}), | ||
| loaderData, | ||
| errors, | ||
| ...(fetchers ? { fetchers } : {}), |
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.
Found this little nuanced bug when copying over the fetcher tests. We need to proxy along any fetchers returned from handleLoaders through to completeNavigation so we get a new state.fetchers instance via the subscribers. Wasn't exposing any user-facing issues though since users can't depend directly on the fetchers map and the internal state.fetchers.get() was always properly updated.
useSubmit/useFetcher to createMemoryRouter
|
On hold until #10413 is merged. This will likely be easiest to just re-do since the changes I think will simplify this a bit. |
|
Closing for now |
Now that #10324 is handled, we can support
useSubmit(and thereforeuseFetcher) inreact-routersince those are no longer coupled toDOM/<form>/FormDataavailability.