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

ShouldRevalidateFunction's actionResult type doesn't match its contents #5759

Closed
1 task done
dmarkow opened this issue Mar 10, 2023 · 1 comment · Fixed by remix-run/react-router#10779
Closed
1 task done
Assignees
Labels
bug Something isn't working feat:typescript

Comments

@dmarkow
Copy link
Contributor

dmarkow commented Mar 10, 2023

What version of Remix are you using?

1.14.1

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

ShouldRevalidateFunction's actionResult argument is of type DataResult, which is defined below. However, actionResult really just holds the data returned by the result.

So if my action returns json({ok: true}) actionData will be {ok: true}. None of the keys from SuccessResult below are actually in the object. And if I return something like json({error: "my error"}, {status: 422}), there's no way to see that status code in my shouldRevalidate function, because all I get is {error: "my error"}. This means I have to bypass the type of actionResult and rely on the shape of the data, instead of using the HTTP status codes to know if my action was successful or not.

export declare enum ResultType {
    data = "data",
    deferred = "deferred",
    redirect = "redirect",
    error = "error"
}
/**
 * Successful result from a loader or action
 */
export interface SuccessResult {
    type: ResultType.data;
    data: any;
    statusCode?: number;
    headers?: Headers;
}
/**
 * Successful defer() result from a loader or action
 */
export interface DeferredResult {
    type: ResultType.deferred;
    deferredData: DeferredData;
    statusCode?: number;
    headers?: Headers;
}
/**
 * Redirect result from a loader or action
 */
export interface RedirectResult {
    type: ResultType.redirect;
    status: number;
    location: string;
    revalidate: boolean;
}
/**
 * Unsuccessful result from a loader or action
 */
export interface ErrorResult {
    type: ResultType.error;
    error: any;
    headers?: Headers;
}
/**
 * Result from a loader or action - potentially successful or unsuccessful
 */
export declare type DataResult = SuccessResult | DeferredResult | RedirectResult | ErrorResult;

Expected Behavior

Either actionResult should just by typed to AppData (which defaults to any) similar to useActionData()'s type, or the result should be returned in the DataResult format.

Actual Behavior

actionResult is of type DataResult which doesn't match it's actual data.

@brophdawg11 brophdawg11 added this to v2 Aug 3, 2023
@brophdawg11 brophdawg11 moved this to Backlog in v2 Aug 3, 2023
@markdalgleish markdalgleish moved this from Backlog to In progress in v2 Aug 11, 2023
@markdalgleish markdalgleish moved this from In progress to PR in v2 Aug 11, 2023
@brophdawg11 brophdawg11 added bug Something isn't working and removed bug:unverified labels Aug 11, 2023
@brophdawg11
Copy link
Contributor

This is fixed by remix-run/react-router#10779 and will be resolved with Remix v2

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Aug 11, 2023
@brophdawg11 brophdawg11 moved this from PR to Merged in v2 Aug 11, 2023
@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feat:typescript
Projects
No open projects
Status: Merged
Development

Successfully merging a pull request may close this issue.

4 participants