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

Fix actionResult type on shouldRevalidate args #10779

Merged
merged 4 commits into from
Aug 11, 2023
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/should-revalidate-action-result-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": patch
---

Fix type for `actionResult` on the arguments object passed to `shouldRevalidate`
47 changes: 26 additions & 21 deletions docs/route/should-revalidate.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,32 @@ new: true

# `shouldRevalidate`

<details>
<summary>Type declaration</summary>

```ts
interface ShouldRevalidateFunction {
(args: ShouldRevalidateFunctionArgs): boolean;
}

interface ShouldRevalidateFunctionArgs {
currentUrl: URL;
currentParams: AgnosticDataRouteMatch["params"];
nextUrl: URL;
nextParams: AgnosticDataRouteMatch["params"];
formMethod?: Submission["formMethod"];
formAction?: Submission["formAction"];
formEncType?: Submission["formEncType"];
text?: Submission["text"];
formData?: Submission["formData"];
json?: Submission["json"];
actionResult?: any;
defaultShouldRevalidate: boolean;
}
```

</details>

This function allows you opt-out of revalidation for a route's loader as an optimization.

<docs-warning>This feature only works if using a data router, see [Picking a Router][pickingarouter]</docs-warning>
Expand Down Expand Up @@ -53,27 +79,6 @@ Note that this is only for data that has already been loaded, is currently rende

<docs-warning>Using this API risks your UI getting out of sync with your data, use with caution!</docs-warning>

## Type Declaration

```ts
interface ShouldRevalidateFunction {
(args: {
currentUrl: URL;
currentParams: AgnosticDataRouteMatch["params"];
nextUrl: URL;
nextParams: AgnosticDataRouteMatch["params"];
formMethod?: Submission["formMethod"];
formAction?: Submission["formAction"];
formEncType?: Submission["formEncType"];
formData?: Submission["formData"];
json?: Submission["json"];
text?: Submission["text"];
actionResult?: DataResult;
defaultShouldRevalidate: boolean;
}): boolean;
}
```

[action]: ./action
[form]: ../components/form
[fetcher]: ../hooks/use-fetcher
Expand Down
26 changes: 16 additions & 10 deletions packages/router/__tests__/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type {
AgnosticNonIndexRouteObject,
AgnosticRouteObject,
DeferredData,
ShouldRevalidateFunctionArgs,
TrackedPromise,
} from "../utils";
import {
Expand Down Expand Up @@ -1860,7 +1861,7 @@ describe("a router", () => {
router.navigate("/params/aValue/bValue");
await tick();
expect(rootLoader.mock.calls.length).toBe(1);
expect(shouldRevalidate.mock.calls[0][0]).toMatchObject({
let expectedArg: ShouldRevalidateFunctionArgs = {
currentParams: {},
currentUrl: expect.URL("http://localhost/child"),
nextParams: {
Expand All @@ -1870,7 +1871,8 @@ describe("a router", () => {
nextUrl: expect.URL("http://localhost/params/aValue/bValue"),
defaultShouldRevalidate: false,
actionResult: undefined,
});
};
expect(shouldRevalidate.mock.calls[0][0]).toMatchObject(expectedArg);
rootLoader.mockClear();
shouldRevalidate.mockClear();

Expand Down Expand Up @@ -1924,7 +1926,7 @@ describe("a router", () => {
expect(shouldRevalidate.mock.calls.length).toBe(1);
// @ts-expect-error
let arg = shouldRevalidate.mock.calls[0][0];
expect(arg).toMatchObject({
let expectedArg: ShouldRevalidateFunctionArgs = {
currentParams: {},
currentUrl: expect.URL("http://localhost/child"),
nextParams: {},
Expand All @@ -1934,7 +1936,8 @@ describe("a router", () => {
formAction: "/child",
formEncType: "application/x-www-form-urlencoded",
actionResult: "ACTION",
});
};
expect(arg).toMatchObject(expectedArg);
// @ts-expect-error
expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" });

Expand Down Expand Up @@ -1977,7 +1980,7 @@ describe("a router", () => {
expect(shouldRevalidate.mock.calls.length).toBe(1);
// @ts-expect-error
let arg = shouldRevalidate.mock.calls[0][0];
expect(arg).toMatchObject({
let expectedArg: ShouldRevalidateFunctionArgs = {
currentParams: {},
currentUrl: expect.URL("http://localhost/child"),
nextParams: {},
Expand All @@ -1987,7 +1990,8 @@ describe("a router", () => {
formAction: "/child",
formEncType: "application/x-www-form-urlencoded",
actionResult: undefined,
});
};
expect(arg).toMatchObject(expectedArg);
// @ts-expect-error
expect(Object.fromEntries(arg.formData)).toEqual({ key: "value" });

Expand Down Expand Up @@ -2022,15 +2026,16 @@ describe("a router", () => {
expect(shouldRevalidate.mock.calls.length).toBe(1);
// @ts-expect-error
let arg = shouldRevalidate.mock.calls[0][0];
expect(arg).toMatchObject({
let expectedArg: Partial<ShouldRevalidateFunctionArgs> = {
formMethod: "post",
formAction: "/",
formEncType: "application/json",
text: undefined,
formData: undefined,
json: { key: "value" },
actionResult: "ACTION",
});
};
expect(arg).toMatchObject(expectedArg);

router.dispose();
});
Expand Down Expand Up @@ -2063,15 +2068,16 @@ describe("a router", () => {
expect(shouldRevalidate.mock.calls.length).toBe(1);
// @ts-expect-error
let arg = shouldRevalidate.mock.calls[0][0];
expect(arg).toMatchObject({
let expectedArg: Partial<ShouldRevalidateFunctionArgs> = {
formMethod: "post",
formAction: "/",
formEncType: "text/plain",
text: "hello world",
formData: undefined,
json: undefined,
actionResult: "ACTION",
});
};
expect(arg).toMatchObject(expectedArg);

router.dispose();
});
Expand Down
4 changes: 2 additions & 2 deletions packages/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import type {
RedirectResult,
RouteData,
RouteManifest,
ShouldRevalidateFunction,
ShouldRevalidateFunctionArgs,
Submission,
SuccessResult,
V7_FormMethod,
Expand Down Expand Up @@ -3513,7 +3513,7 @@ function isNewRouteInstance(

function shouldRevalidateLoader(
loaderMatch: AgnosticDataRouteMatch,
arg: Parameters<ShouldRevalidateFunction>[0]
arg: ShouldRevalidateFunctionArgs
) {
if (loaderMatch.route.shouldRevalidate) {
let routeChoice = loaderMatch.route.shouldRevalidate(arg);
Expand Down
33 changes: 19 additions & 14 deletions packages/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,24 @@ export interface ActionFunction {
(args: ActionFunctionArgs): Promise<DataFunctionValue> | DataFunctionValue;
}

/**
* Arguments passed to shouldRevalidate function
*/
export interface ShouldRevalidateFunctionArgs {
currentUrl: URL;
currentParams: AgnosticDataRouteMatch["params"];
nextUrl: URL;
nextParams: AgnosticDataRouteMatch["params"];
formMethod?: Submission["formMethod"];
formAction?: Submission["formAction"];
formEncType?: Submission["formEncType"];
text?: Submission["text"];
formData?: Submission["formData"];
json?: Submission["json"];
actionResult?: any;
defaultShouldRevalidate: boolean;
}

/**
* Route shouldRevalidate function signature. This runs after any submission
* (navigation or fetcher), so we flatten the navigation/fetcher submission
Expand All @@ -182,20 +200,7 @@ export interface ActionFunction {
* have to re-run based on the data models that were potentially mutated.
*/
export interface ShouldRevalidateFunction {
(args: {
currentUrl: URL;
currentParams: AgnosticDataRouteMatch["params"];
nextUrl: URL;
nextParams: AgnosticDataRouteMatch["params"];
formMethod?: Submission["formMethod"];
formAction?: Submission["formAction"];
formEncType?: Submission["formEncType"];
text?: Submission["text"];
formData?: Submission["formData"];
json?: Submission["json"];
actionResult?: DataResult;
defaultShouldRevalidate: boolean;
}): boolean;
(args: ShouldRevalidateFunctionArgs): boolean;
}

/**
Expand Down