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(remix-react): add submitOptions argument for useSubmit and useFetcher #4882

Closed
wants to merge 10 commits into from
6 changes: 6 additions & 0 deletions .changeset/metal-bikes-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"remix": minor
"@remix-run/react": minor
---

feat(remix-react): add `submitOptions` argument for `useSubmit` and `useFetcher`
kentcdodds marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 18 additions & 0 deletions docs/hooks/use-fetcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ Notes about how it works:
- Handles uncaught errors by rendering the nearest `ErrorBoundary` (just like a normal navigation from `<Link>` or `<Form>`)
- Will redirect the app if your action/loader being called returns a redirect (just like a normal navigation from `<Link>` or `<Form>`)

### Arguments

`useFetcher` takes one optional argument which is the `submitOptions` that will be used as a default for calls to `fetcher.submit`. This is useful to avoid repetition and to ensure the reference to `fetcher` remains stable if you need to include it in a dependency array.

#### `submitOptions.method`

The URL method to use when submitting. Defaults to `"get"` (options are `"get"`, `"post"`, `"put"`, `"patch"`, `"delete"`).

#### `submitOptions.action`

The URL to submit to. Defaults to the current URL.

#### `submitOptions.encType`

The `enctype` to use when submitting. Defaults to `"application/x-www-form-urlencoded"`.

### Properties

#### `fetcher.state`

You can know the state of the fetcher with `fetcher.state`. It will be one of:
Expand Down
20 changes: 20 additions & 0 deletions docs/hooks/use-submit.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,23 @@ function useSessionTimeout() {
}, [submit, transition]);
}
```

### Arguments

`useSubmit` takes one optional argument which is the `submitOptions` that will be used as a default for calls to `submit`. This is useful to avoid repetition and to ensure the reference to `submit` remains stable if you need to include it in a dependency array.

#### `submitOptions.method`

The URL method to use when submitting. Defaults to `"get"` (options are `"get"`, `"post"`, `"put"`, `"patch"`, `"delete"`).

#### `submitOptions.action`

The URL to submit to. Defaults to the current URL.

#### `submitOptions.encType`

The `enctype` to use when submitting. Defaults to `"application/x-www-form-urlencoded"`.

#### `submitOptions.replace`

Whether to replace the current history entry or push a new one. Defaults to `false`.
39 changes: 38 additions & 1 deletion integration/fetcher-test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test } from "@playwright/test";
import { test, expect } from "@playwright/test";

import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
import type { Fixture, AppFixture } from "./helpers/create-fixture";
Expand Down Expand Up @@ -143,6 +143,34 @@ test.describe("useFetcher", () => {
);
}
`,

"app/routes/stable-fetcher.jsx": js`
import { useFetcher, useSearchParams } from "@remix-run/react";
import { useEffect, useRef } from "react";

export default function Fetcher() {
const [, setSearchParams] = useSearchParams();
const clickedRef = useRef(false);
const fetcher = useFetcher({ action: "/whatever" });

useEffect(() => {
if (clickedRef.current) {
throw new Error("fetcher changed!");
}
}, [fetcher]);

return (
<button
onClick={() => {
clickedRef.current = true;
setSearchParams({ random: Math.random().toString() });
}}
>
Set searchParams
</button>
);
}
`,
},
});

Expand Down Expand Up @@ -232,4 +260,13 @@ test.describe("useFetcher", () => {
await app.clickElement("#submit-index-post");
await page.waitForSelector(`pre:has-text("${PARENT_INDEX_ACTION}")`);
});

test("fetchers should be stable when providing submit options", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/stable-fetcher");
await app.clickElement("button");
expect(await app.getHtml()).not.toContain("fetcher changed!");
});
});
55 changes: 40 additions & 15 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1256,16 +1256,29 @@ export interface SubmitFunction {
*
* @see https://remix.run/api/remix#usesubmit
*/
export function useSubmit(): SubmitFunction {
return useSubmitImpl();
export function useSubmit(options?: SubmitOptions): SubmitFunction {
let key = undefined; // no key for global submit
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about making key a part of the options object in useSubmitImpl instead of accepting it as a first param, but decided to not cause a breaking change to useSubmitImpl because it's exported and it's possible people are using it. I actually don't hate this though so maybe it's fine?

Copy link
Member

Choose a reason for hiding this comment

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

Since I think we can safely remove the export (as it was never intended to be a public API), I think we can make key part of the submitOptions

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually kinda like that submitOptions is a separate argument, even though we have this funny key = undefined thing. But I don't care enough.

return useSubmitImpl(key, options);
}

let defaultMethod = "get";
let defaultEncType = "application/x-www-form-urlencoded";

export function useSubmitImpl(key?: string): SubmitFunction {
const DEFAULT_METHOD = "get";
const DEFAULT_ENC_TYPE = "application/x-www-form-urlencoded";
Comment on lines +1264 to +1265
Copy link
Member Author

Choose a reason for hiding this comment

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

Switched these to const and upper-case because otherwise typescript wasn't happy about default-assigning submitMethod to DEFAULT_METHOD below. Using const here allows typescript to narrow this down from a type of string to "get" which fits in the FormMethod type.


// TODO: remove the export here?
export function useSubmitImpl(
key?: string,
{
method: submitMethod = DEFAULT_METHOD,
action: submitAction,
encType: submitEncType = DEFAULT_ENC_TYPE,
replace,
}: SubmitOptions = {}
): SubmitFunction {
let navigate = useNavigate();
let defaultAction = useFormAction();
let defaultFormAction = useFormAction();
let defaultAction = submitAction || defaultFormAction;
let defaultMethod = submitMethod || DEFAULT_METHOD;
let defaultEncType = submitEncType || DEFAULT_ENC_TYPE;
Comment on lines +1278 to +1281
Copy link
Member Author

Choose a reason for hiding this comment

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

By putting these calculations all here we can avoid having to pass defaultFormAction into the dependency array which is what fixes my original issue.

let { transitionManager } = useRemixEntryContext();

return React.useCallback(
Expand Down Expand Up @@ -1334,9 +1347,9 @@ export function useSubmitImpl(key?: string): SubmitFunction {
);
}

method = options.method || "get";
method = options.method || defaultMethod;
action = options.action || defaultAction;
encType = options.encType || "application/x-www-form-urlencoded";
encType = options.encType || defaultEncType;

if (target instanceof FormData) {
formData = target;
Expand Down Expand Up @@ -1408,10 +1421,21 @@ export function useSubmitImpl(key?: string): SubmitFunction {
});
} else {
setNextNavigationSubmission(submission);
navigate(url.pathname + url.search, { replace: options.replace });
navigate(url.pathname + url.search, {
replace:
typeof options.replace === "undefined" ? replace : options.replace,
MichaelDeBoey marked this conversation as resolved.
Show resolved Hide resolved
});
}
},
[defaultAction, key, navigate, transitionManager]
[
defaultAction,
defaultEncType,
defaultMethod,
key,
navigate,
replace,
transitionManager,
]
);
}

Expand Down Expand Up @@ -1578,17 +1602,18 @@ export type FetcherWithComponents<TData> = Fetcher<TData> & {
*
* @see https://remix.run/api/remix#usefetcher
*/
export function useFetcher<TData = any>(): FetcherWithComponents<
SerializeFrom<TData>
> {
export function useFetcher<TData = any>(
// replace doesn't make sense for fetchers
submitOptions?: Omit<SubmitOptions, "replace">
): FetcherWithComponents<SerializeFrom<TData>> {
let { transitionManager } = useRemixEntryContext();

let [key] = React.useState(() => String(++fetcherId));
let [Form] = React.useState(() => createFetcherForm(key));
let [load] = React.useState(() => (href: string) => {
transitionManager.send({ type: "fetcher", href, key });
});
let submit = useSubmitImpl(key);
let submit = useSubmitImpl(key, submitOptions);

let fetcher = transitionManager.getFetcher<SerializeFrom<TData>>(key);

Expand Down