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: GET forms now expose a submission on the loading navigation #9695

Merged
merged 2 commits into from
Dec 7, 2022
Merged
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
5 changes: 5 additions & 0 deletions .changeset/thin-kids-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

GET forms now expose a submission on the loading navigation
52 changes: 38 additions & 14 deletions packages/router/__tests__/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3372,9 +3372,9 @@ describe("a router", () => {
});
let navigation = t.router.state.navigation;
expect(navigation.state).toBe("loading");
expect(navigation.formData).toBeUndefined();
expect(navigation.formMethod).toBeUndefined();
expect(navigation.formEncType).toBeUndefined();
expect(navigation.formData).toEqual(createFormData({ gosh: "dang" }));
expect(navigation.formMethod).toBe("get");
expect(navigation.formEncType).toBe("application/x-www-form-urlencoded");
expect(navigation.location).toMatchObject({
pathname: "/foo",
search: "?gosh=dang",
Expand All @@ -3400,9 +3400,9 @@ describe("a router", () => {

let navigation = t.router.state.navigation;
expect(navigation.state).toBe("loading");
expect(navigation.formData).toBeUndefined();
expect(navigation.formMethod).toBeUndefined();
expect(navigation.formEncType).toBeUndefined();
expect(navigation.formData).toEqual(createFormData({ gosh: "dang" }));
expect(navigation.formMethod).toBe("get");
expect(navigation.formEncType).toBe("application/x-www-form-urlencoded");
expect(navigation.location?.pathname).toBe("/bar");

await B.loaders.bar.resolve("B");
Expand Down Expand Up @@ -3872,8 +3872,10 @@ describe("a router", () => {
pathname: "/tasks",
search: "?key=value",
});
expect(t.router.state.navigation.formMethod).toBeUndefined();
expect(t.router.state.navigation.formData).toBeUndefined();
expect(t.router.state.navigation.formMethod).toBe("get");
expect(t.router.state.navigation.formData).toEqual(
createFormData({ key: "value" })
);
});

it("converts formData to URLSearchParams for formMethod=get", async () => {
Expand All @@ -3890,8 +3892,10 @@ describe("a router", () => {
pathname: "/tasks",
search: "?key=value",
});
expect(t.router.state.navigation.formMethod).toBeUndefined();
expect(t.router.state.navigation.formData).toBeUndefined();
expect(t.router.state.navigation.formMethod).toBe("get");
expect(t.router.state.navigation.formData).toEqual(
createFormData({ key: "value" })
);
});

it("does not preserve existing 'action' URLSearchParams for formMethod='get'", async () => {
Expand All @@ -3908,8 +3912,10 @@ describe("a router", () => {
pathname: "/tasks",
search: "?key=2",
});
expect(t.router.state.navigation.formMethod).toBeUndefined();
expect(t.router.state.navigation.formData).toBeUndefined();
expect(t.router.state.navigation.formMethod).toBe("get");
expect(t.router.state.navigation.formData).toEqual(
createFormData({ key: "2" })
);
});

it("preserves existing 'action' URLSearchParams for formMethod='post'", async () => {
Expand Down Expand Up @@ -4656,6 +4662,20 @@ describe("a router", () => {
signal: nav3.loaders.tasks.stub.mock.calls[0][0].request.signal,
}),
});

let nav4 = await t.navigate("/tasks#hash", {
formData: createFormData({ foo: "bar" }),
});
expect(nav4.loaders.tasks.stub).toHaveBeenCalledWith({
params: {},
request: new Request("http://localhost/tasks?foo=bar", {
signal: nav4.loaders.tasks.stub.mock.calls[0][0].request.signal,
}),
});

expect(t.router.state.navigation.formAction).toBe("/tasks");
expect(t.router.state.navigation?.location?.pathname).toBe("/tasks");
expect(t.router.state.navigation?.location?.search).toBe("?foo=bar");
});

it("handles errors thrown from loaders", async () => {
Expand Down Expand Up @@ -6165,8 +6185,8 @@ describe("a router", () => {
pathname: "/tasks",
search: "?key=value",
},
formMethod: undefined,
formData: undefined,
formMethod: "get",
formData: createFormData({ key: "value" }),
},
revalidation: "loading",
loaderData: {
Expand Down Expand Up @@ -6933,6 +6953,10 @@ describe("a router", () => {
formData: createFormData({ key: "value" }),
});
expect(A.fetcher.state).toBe("loading");
expect(A.fetcher.formMethod).toBe("get");
expect(A.fetcher.formAction).toBe("/foo");
expect(A.fetcher.formData).toEqual(createFormData({ key: "value" }));
expect(A.fetcher.formEncType).toBe("application/x-www-form-urlencoded");
expect(
new URL(
A.loaders.foo.stub.mock.calls[0][0].request.url
Expand Down
67 changes: 41 additions & 26 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type {
Submission,
SuccessResult,
AgnosticRouteMatch,
SubmissionFormMethod,
MutationFormMethod,
} from "./utils";
import {
DeferredData,
Expand Down Expand Up @@ -515,15 +515,20 @@ interface QueryRouteResponse {
response: Response;
}

const validActionMethodsArr: SubmissionFormMethod[] = [
const validMutationMethodsArr: MutationFormMethod[] = [
"post",
"put",
"patch",
"delete",
];
const validActionMethods = new Set<SubmissionFormMethod>(validActionMethodsArr);
const validMutationMethods = new Set<MutationFormMethod>(
validMutationMethodsArr
);

const validRequestMethodsArr: FormMethod[] = ["get", ...validActionMethodsArr];
const validRequestMethodsArr: FormMethod[] = [
"get",
...validMutationMethodsArr,
];
const validRequestMethods = new Set<FormMethod>(validRequestMethodsArr);

const redirectStatusCodes = new Set([301, 302, 303, 307, 308]);
Expand Down Expand Up @@ -805,7 +810,8 @@ export function createRouter(init: RouterInit): Router {
};

let historyAction =
(opts && opts.replace) === true || submission != null
(opts && opts.replace) === true ||
(submission != null && isMutationMethod(submission.formMethod))
? HistoryAction.Replace
: HistoryAction.Push;
let preventScrollReset =
Expand Down Expand Up @@ -929,7 +935,11 @@ export function createRouter(init: RouterInit): Router {
pendingError = {
[findNearestBoundary(matches).route.id]: opts.pendingError,
};
} else if (opts && opts.submission) {
} else if (
opts &&
opts.submission &&
isMutationMethod(opts.submission.formMethod)
) {
// Call action if we received an action submission
let actionOutput = await handleAction(
request,
Expand Down Expand Up @@ -1089,6 +1099,7 @@ export function createRouter(init: RouterInit): Router {
formAction: undefined,
formEncType: undefined,
formData: undefined,
...submission,
};
loadingNavigation = navigation;
}
Expand Down Expand Up @@ -1253,15 +1264,15 @@ export function createRouter(init: RouterInit): Router {
let { path, submission } = normalizeNavigateOptions(href, opts, true);
let match = getTargetMatch(matches, path);

if (submission) {
if (submission && isMutationMethod(submission.formMethod)) {
handleFetcherAction(key, routeId, path, match, matches, submission);
return;
}

// Store off the match so we can call it's shouldRevalidate on subsequent
// revalidations
fetchLoadMatches.set(key, [path, match, matches]);
handleFetcherLoader(key, routeId, path, match, matches);
handleFetcherLoader(key, routeId, path, match, matches, submission);
}

// Call the action for the matched fetcher.submit(), and then handle redirects,
Expand Down Expand Up @@ -1484,7 +1495,8 @@ export function createRouter(init: RouterInit): Router {
routeId: string,
path: string,
match: AgnosticDataRouteMatch,
matches: AgnosticDataRouteMatch[]
matches: AgnosticDataRouteMatch[],
submission?: Submission
) {
let existingFetcher = state.fetchers.get(key);
// Put this fetcher into it's loading state
Expand All @@ -1494,6 +1506,7 @@ export function createRouter(init: RouterInit): Router {
formAction: undefined,
formEncType: undefined,
formData: undefined,
...submission,
data: existingFetcher && existingFetcher.data,
};
state.fetchers.set(key, loadingFetcher);
Expand Down Expand Up @@ -1625,12 +1638,12 @@ export function createRouter(init: RouterInit): Router {
let { formMethod, formAction, formEncType, formData } = state.navigation;

// If this was a 307/308 submission we want to preserve the HTTP method and
// re-submit the POST/PUT/PATCH/DELETE as a submission navigation to the
// re-submit the GET/POST/PUT/PATCH/DELETE as a submission navigation to the
// redirected location
if (
redirectPreserveMethodStatusCodes.has(redirect.status) &&
formMethod &&
isSubmissionMethod(formMethod) &&
isMutationMethod(formMethod) &&
formEncType &&
formData
) {
Expand Down Expand Up @@ -2072,7 +2085,7 @@ export function unstable_createStaticHandler(
);

try {
if (isSubmissionMethod(request.method.toLowerCase())) {
if (isMutationMethod(request.method.toLowerCase())) {
let result = await submit(
request,
matches,
Expand Down Expand Up @@ -2369,17 +2382,19 @@ function normalizeNavigateOptions(
}

// Create a Submission on non-GET navigations
if (opts.formMethod && isSubmissionMethod(opts.formMethod)) {
return {
path,
submission: {
formMethod: opts.formMethod,
formAction: stripHashFromPath(path),
formEncType:
(opts && opts.formEncType) || "application/x-www-form-urlencoded",
formData: opts.formData,
},
let submission: Submission | undefined;
if (opts.formData) {
submission = {
formMethod: opts.formMethod || "get",
formAction: stripHashFromPath(path),
formEncType:
(opts && opts.formEncType) || "application/x-www-form-urlencoded",
formData: opts.formData,
};

if (isMutationMethod(submission.formMethod)) {
return { path, submission };
}
}

// Flatten submission onto URLSearchParams for GET submissions
Expand All @@ -2404,7 +2419,7 @@ function normalizeNavigateOptions(
};
}

return { path: createPath(parsedPath) };
return { path: createPath(parsedPath), submission };
}

// Filter out all routes below any caught error as they aren't going to
Expand Down Expand Up @@ -2721,7 +2736,7 @@ function createRequest(
let url = createURL(stripHashFromPath(location)).toString();
let init: RequestInit = { signal };

if (submission) {
if (submission && isMutationMethod(submission.formMethod)) {
let { formMethod, formEncType, formData } = submission;
init.method = formMethod.toUpperCase();
init.body =
Expand Down Expand Up @@ -3061,8 +3076,8 @@ function isValidMethod(method: string): method is FormMethod {
return validRequestMethods.has(method as FormMethod);
}

function isSubmissionMethod(method: string): method is SubmissionFormMethod {
return validActionMethods.has(method as SubmissionFormMethod);
function isMutationMethod(method?: string): method is MutationFormMethod {
return validMutationMethods.has(method as MutationFormMethod);
}

async function resolveDeferredResults(
Expand Down
6 changes: 3 additions & 3 deletions packages/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export type DataResult =
| RedirectResult
| ErrorResult;

export type SubmissionFormMethod = "post" | "put" | "patch" | "delete";
export type FormMethod = "get" | SubmissionFormMethod;
export type MutationFormMethod = "post" | "put" | "patch" | "delete";
export type FormMethod = "get" | MutationFormMethod;

export type FormEncType =
| "application/x-www-form-urlencoded"
Expand All @@ -75,7 +75,7 @@ export type FormEncType =
* external consumption
*/
export interface Submission {
formMethod: SubmissionFormMethod;
formMethod: FormMethod;
formAction: string;
formEncType: FormEncType;
formData: FormData;
Expand Down