Skip to content

Commit

Permalink
Fix actionResult type on shouldRevalidate args (#10779)
Browse files Browse the repository at this point in the history
* Fix actionResult type on shouldRevalidate args

* rename type for consistency

* use args type in shouldRevalidateLoader

* Move type declaration into details/summary

---------

Co-authored-by: Matt Brophy <matt@brophy.org>
  • Loading branch information
markdalgleish and brophdawg11 authored Aug 11, 2023
1 parent 6a08757 commit e11af30
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 47 deletions.
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

0 comments on commit e11af30

Please sign in to comment.