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

feat(react-router): add useSubmit/useFetcher to createMemoryRouter #10390

Closed
wants to merge 5 commits into from

Conversation

brophdawg11
Copy link
Contributor

Now that #10324 is handled, we can support useSubmit (and therefore useFetcher) in react-router since those are no longer coupled to DOM/<form>/FormData availability.

@changeset-bot
Copy link

changeset-bot bot commented Apr 24, 2023

🦋 Changeset detected

Latest commit: b0a66bd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
react-router Minor
react-router-dom Minor
react-router-dom-v5-compat Minor
react-router-native Minor

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

@@ -941,6 +949,180 @@ function useNavigateStable(): NavigateFunction {
return navigate;
}

type SubmitPayload = NonNullable<unknown> | null;
Copy link
Contributor Author

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

Comment on lines +1024 to +1030
// Base options shared between fetch() and navigate()
let opts = {
payload,
formMethod: options.method || "get",
formEncType: options.encType,
action: routerAction,
};
Copy link
Contributor Author

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

Comment on lines +1052 to +1060
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;
};
Copy link
Contributor Author

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(
Copy link
Contributor Author

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.

@@ -1361,6 +1365,7 @@ export function createRouter(init: RouterInit): Router {
...(pendingActionData ? { actionData: pendingActionData } : {}),
loaderData,
errors,
...(fetchers ? { fetchers } : {}),
Copy link
Contributor Author

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.

@MichaelDeBoey MichaelDeBoey changed the title Add useSubmit/useFetcher to createMemoryRouter feat(react-router): add useSubmit/useFetcher to createMemoryRouter Apr 28, 2023
@brophdawg11
Copy link
Contributor Author

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.

@brophdawg11
Copy link
Contributor Author

Closing for now

@brophdawg11 brophdawg11 closed this Jun 7, 2023
@MichaelDeBoey MichaelDeBoey deleted the brophdawg11/memory-submit branch July 11, 2023 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants