Skip to content

Commit

Permalink
fix(remix-react): submit <Form> w/method set by submitter formmethod
Browse files Browse the repository at this point in the history
Fix `<Form>` component by not passing the form's `method` into the
`options` when calling the `submit(target, options)` function (created
with the `useSubmit()` hook). This ensures that the option does not take
precedence on any `formethod` set on the submitter which otherwise break
the `formmethod` functionality.

Since #3094 and #3094 the `<Form>` passes the "submitter" (if any) as
the `target` to the `submit(target, options)` (`useSubmitImpl`) which
will attempt to read the `formmethod`, `formaction` and `formenctype`
attributes on the target. Therefore, the responsability to infer which
method should be used should solely be on `useSubmitImpl` for the given
`target`.

Co-authored-by: Maxime Doury <[email protected]>
  • Loading branch information
nrako and Maxime Doury committed Jun 30, 2022
1 parent 06333c1 commit 9360e17
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 30 deletions.
58 changes: 29 additions & 29 deletions integration/bug-report-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,34 +48,29 @@ test.beforeAll(async () => {
////////////////////////////////////////////////////////////////////////////
files: {
"app/routes/index.jsx": js`
import { useActionData, useLoaderData, Form } from "@remix-run/react";
import { json } from '@remix-run/server-runtime'
import { json } from "@remix-run/node";
import { useLoaderData, Link } from "@remix-run/react";
export function action({ request }) {
return json(request.method)
}
export function loader({ request }) {
return json(request.method)
export function loader() {
return json("pizza");
}
export default function Index() {
let actionData = useActionData();
let loaderData = useLoaderData();
let data = useLoaderData();
return (
<>
<Form method="post">
<button type="submit" formMethod="get">Submit with GET</button>
</Form>
<form method="get">
<button type="submit" formMethod="post">Submit with POST</button>
</form>
<pre>{loaderData || actionData}</pre>
</>
<div>
{data}
<Link to="/burgers">Other Route</Link>
</div>
)
}
`,

"app/routes/burgers.jsx": js`
export default function Index() {
return <div>cheeseburger</div>;
}
`,
},
});

Expand All @@ -90,17 +85,22 @@ test.afterAll(async () => appFixture.close());
// add a good description for what you expect Remix to do 👇🏽
////////////////////////////////////////////////////////////////////////////////

test("`<Form>` should submit with the method set via the `formmethod` attribute set on the submitter (button)", async ({
page,
}) => {
test("[description of what you expect it to do]", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
// You can test any request your app might get using `fixture`.
let response = await fixture.requestDocument("/");
expect(await response.text()).toMatch("pizza");

// If you need to test interactivity use the `app`
await app.goto("/");
await app.clickElement("text=Submit with GET");
await page.waitForLoadState("load");
expect(await app.getHtml("pre")).toBe(`<pre>GET</pre>`);
await page.waitForLoadState("load");
await app.clickElement("text=Submit with POST");
expect(await app.getHtml("pre")).toBe(`<pre>POST</pre>`);
await app.clickLink("/burgers");
expect(await app.getHtml()).toMatch("cheeseburger");

// If you're not sure what's going on, you can "poke" the app, it'll
// automatically open up in your browser for 20 seconds, so be quick!
// await app.poke(20);

// Go check out the other tests to see what else you can do.
});

////////////////////////////////////////////////////////////////////////////////
Expand Down
48 changes: 48 additions & 0 deletions integration/form-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,36 @@ test.describe("Forms", () => {
)
}
`,

"app/routes/submitter-formmethod.jsx": js`
import { useActionData, useLoaderData, Form } from "@remix-run/react";
import { json } from '@remix-run/server-runtime'
export function action({ request }) {
return json(request.method)
}
export function loader({ request }) {
return json(request.method)
}
export default function Index() {
let actionData = useActionData();
let loaderData = useLoaderData();
return (
<>
<Form method="post">
<button type="submit" formMethod="get">Submit with GET</button>
</Form>
<Form method="get">
<button type="submit" formMethod="post">Submit with POST</button>
</Form>
<pre>{actionData || loaderData}</pre>
</>
)
}
`,
},
});

Expand Down Expand Up @@ -575,4 +605,22 @@ test.describe("Forms", () => {
});
});
});

test.describe("with submitter button having `formMethod` attribute", () => {
test("submits with GET instead of POST", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/submitter-formmethod");
await app.clickElement("text=Submit with GET");
await page.waitForLoadState("load");
expect(await app.getHtml("pre")).toBe("<pre>GET</pre>");
});

test("submits with POST instead of GET", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/submitter-formmethod");
await app.clickElement("text=Submit with POST");
await page.waitForLoadState("load");
expect(await app.getHtml("pre")).toBe("<pre>POST</pre>");
});
});
});
2 changes: 1 addition & 1 deletion packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
let submitter = (event as unknown as HTMLSubmitEvent)
.nativeEvent.submitter as HTMLFormSubmitter | null;

submit(submitter || event.currentTarget, { method, replace });
submit(submitter || event.currentTarget, { replace });
}
}
{...props}
Expand Down

0 comments on commit 9360e17

Please sign in to comment.