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

tests(remix-react): add failing test for default Form action #3211

Closed
wants to merge 4 commits into from
Closed
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
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
- ianduvall
- illright
- imzshh
- ionut-botizan
- isaacrmoreno
- ishan-me
- IshanKBG
Expand Down
41 changes: 41 additions & 0 deletions integration/form-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ test.describe("Forms", () => {
let SPLAT_ROUTE_CURRENT_ACTION = "splat-route-cur";
let SPLAT_ROUTE_PARENT_ACTION = "splat-route-parent";
let SPLAT_ROUTE_TOO_MANY_DOTS_ACTION = "splat-route-too-many-dots";
let UNDEFINED_ACTION_SEARCH_PARAMS = "undefined-action-search-params";
let EXPLICIT_ACTION_SEARCH_PARAMS = "explicit-action-search-params";

test.beforeAll(async () => {
fixture = await createFixture({
Expand Down Expand Up @@ -277,6 +279,22 @@ test.describe("Forms", () => {
)
}
`,

"app/routes/login.jsx": js`
import { Form } from "@remix-run/react";
export default function() {
return (
<>
<Form id="${UNDEFINED_ACTION_SEARCH_PARAMS}">
<button>Login</button>
</Form>
<Form id="${EXPLICIT_ACTION_SEARCH_PARAMS}" action="/auth?nonce=123">
<button>Login</button>
</Form>
</>
)
}
`,
},
});

Expand Down Expand Up @@ -574,5 +592,28 @@ test.describe("Forms", () => {
expect(el.attr("action")).toMatch("/");
});
});

test.describe("in a route with search params", () => {
test("undefined action preserves current location's search params", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
let url = "/login?redirectTo=/profile";
await app.goto(url);
let html = await app.getHtml();
let el = getElement(html, `#${UNDEFINED_ACTION_SEARCH_PARAMS}`);
expect(el.attr("action")).toMatch(url);
});

test("explicit action is preserved as defined", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
let url = "/login?redirectTo=/profile";
let action = "/auth?nonce=123";
await app.goto(url);
let html = await app.getHtml();
let el = getElement(html, `#${EXPLICIT_ACTION_SEARCH_PARAMS}`);
expect(el.attr("action")).toMatch(action);
});
});
});
});
13 changes: 9 additions & 4 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
reloadDocument = false,
replace = false,
method = "get",
action = ".",
action,
encType = "application/x-www-form-urlencoded",
fetchKey,
onSubmit,
Expand Down Expand Up @@ -988,16 +988,21 @@ type HTMLFormSubmitter = HTMLButtonElement | HTMLInputElement;
* @see https://remix.run/api/remix#useformaction
*/
export function useFormAction(
action = ".",
action?: string,
// TODO: Remove method param in v2 as it's no longer needed and is a breaking change
method: FormMethod = "get"
): string {
let { id } = useRemixRouteContext();
let path = useResolvedPath(action);
let path = useResolvedPath(action ?? ".");
let location = useLocation();
let search = path.search;
let isIndexRoute = id.endsWith("/index");

if (action === "." && isIndexRoute) {
if (action === undefined) {
search = location.search;
}

if ((action === undefined || action === ".") && isIndexRoute) {
search = search ? search.replace(/^\?/, "?index&") : "?index";
}

Expand Down