SafeActionResult returns undefined

[ScreamZ]

I don't understand why SafeActionResult might be undefined using direct execution ? This wasn't the case in v6

"use server";
import { createSafeActionClient } from "next-safe-action";
import { z } from "zod";

const test = createSafeActionClient();

export const testAction = test.action(async () => ({
  content: "hello world",
}));

Now using this inside another file the result can be undefined.
I don't understand why ? And also this prevent destructiring

[TheEdoRan]

Hi, that's because if you use next/navigation functions like redirect() inside middleware functions or action's server code function, the result of the action is undefined. Those functions work by throwing a Next.js internal error that gets then caught by the framework. So the type is actually correct now.

[ScreamZ]

I was thinking about this too.

"use server";
import { createSafeActionClient } from "next-safe-action";
import { z } from "zod";

const actionClient = createSafeActionClient();

// This schema is used to validate input from client.
const schema = z.object({
  username: z.string().min(3).max(10),
  password: z.string().min(8).max(100),
});

export const loginUser = actionClient
  .schema(schema)
  .action(async ({ parsedInput: { username, password } }) => {
    if (username === "johndoe" && password === "123456") {
      return {
        success: "Successfully logged in",
      };
    }

    return { failure: "Incorrect credentials" };
  });

(async () => {
  const result = await loginUser({ password: "", username: "" });
  // Still possible undefined
})();

So briefly what you meant is that is something is thrown, instead of trigger server error, it returns undefined, that's it ?

[TheEdoRan]

By default every server error gets caught by the library and a serverError is returned. The only exception is for Next.js navigation errors, which are instead caught by the framework. If you use redirect the result will be undefined. TypeScript cannot infer a never return, so the safest way for the end user is to type the result as object | undefined, as it is right now.

[ScreamZ]

Alright, i'll avoid destructuring without undefined checks then, thanks

[TheEdoRan]

You could use a ! to type it as non-undefined, but unfortunately I can't do anything about this, since Next.js + TypeScript work this way, the type should have been this since the beginning, which is my bad. I understand it's a little worse DX, but it's also safer now.

[ScreamZ]

i'll make a convenient function that wrap a call and throw an error if needed maybe. I understand your point

[laurent512]

Same problem I guess,
I have to do something like this which indeed is very inconvenient , is there an alternative ?

Maybe this occur because I use i18n library?

[laurent512]

Or something like this : const propertyDetails = (await getPropertyDetails({ propertyId }))?.data ?? undefined;
It was more convenient before to be honest :)

[laurent512]

Or more generalized this :

 async function fetchAndExtract<T, K extends keyof T>(
    fetchFunction: () => Promise<T | null | undefined>,
    property: K
): Promise<T[K] | undefined> {
    const result = await fetchFunction();
    return result?.[property] ?? undefined;
}

// Usage example
const propertyDetails = await fetchAndExtract(
    () => getPropertyDetails({ propertyId }),
    'data'
);

[TheEdoRan]

@laurent512 can't you just write it like this?

const { data } = (await getPropertyDetails({ propertyId }))!;

Unfortunately, as I already explained above, the correct typing for the result is object | undefined, because if you use redirect inside the action's body you receive undefined back on the client.

[laurent512]

Oh yes it works, my solution was a bit too complicated ^^'
As an alternative, do you think it would be possible to have a flag in the safe action client to globally disable that type error ?

[hannanstd]

it works but it is a little dirty doing it on every action

[AlwanN01]

@TheEdoRan I know the result using next redirect is undefined, but wouldn't using redirect on a regular server action not return undefined as a type? the official documentation does not discuss this. Isn't this getting confusing?
because it is very annoying dx I did not use this library.

[TheEdoRan]

wouldn't using redirect on a regular server action not return undefined as a type?

No, because redirect throws an error that gets caught by Next.js to actually do the redirect. Problem is that if you call the Server Action from a Client Component and use redirect inside the action's body, you get back undefined as the result of the action after the redirect. Not sure if this should happen or not, but Next.js works this way as of now.

[VirtuaBoza]

@TheEdoRan would it be possible not execute the onSuccess callback in this scenario?

[eug-vs]

@VirtuaBoza really great idea, I think that would absolutely solve this issue, giving us all the benefits of good DX and onSuccess not triggering on redirects makes a lot of sense anyway.

[TheEdoRan]

@VirtuaBoza @eug-vs I understand your point of view, but I have some concerns about changing the onSuccess hook callback.

1. This has been the behavior since the release of next-safe-action. The onSuccess callback is always run after a redirect, and there's probably production code out there that relies on this behavior (which, by the way, is intentional, as explained in 2.). Changing it could possibly be breaking.
2. Why shouldn't redirection be considered a success? In my opinion, if the action finishes its execution with a navigation, it should be considered successful, and in some cases it's very useful to display information after a redirect, for instance using a toast.

Moreover, you can just check for typeof data === "undefined" inside onSuccess (assuming you're returning something from the Server Action when not redirecting) to prevent execution of the callback.

Hope this will clear up some confusion. Please let me know what you think as well, thanks.

[eug-vs]

Thanks for the explanation @TheEdoRan! I agree that this would be too much of a breaking change, and redirection is in fact a success as you pointed out. Checking typeof data === 'undefined' is a bit annoying if we have to do it every time, but we can always do a tiny wrapper to handle that in specific cases, and the reasons for it are convincing.

[davidchalifoux]

I threw together this helper function to handle this. It throws an error if the action response or response data are undefined. Feel free to modify to your needs!

/**
 * Helper function to use an action on the server for data fetching.
 * Catches undefined responses.
 * @throws Error
 */
export async function handleActionResult<
  ServerError,
  S extends Schema,
  BAS extends readonly Schema[],
  CVE,
  CBAVE,
  Data,
  NextCtx,
>(
  action: Promise<
    SafeActionResult<ServerError, S, BAS, CVE, CBAVE, Data, NextCtx> | undefined
  >
) {
  const res = await action;

  if (typeof res === "undefined") {
    throw new Error("Action returned with undefined result.");
  }

  const data = res.data;

  if (typeof data === "undefined") {
    throw new Error("Action returned with data undefined.");
  }

  return { ...res, data };
}

Usage:

const brands = await handleActionResult(getVendorBrands());

[ScreamZ]

Thanks Nice but sometimes redirection is expected, we could just opt for no-op also. :)

[Lermatroid]

Hey all!

Running into this as well at the moment (Latest version of next & next-safe-action). I have action which down every path returns a object, even going as far as to create a "dummy object" which pending validation errors returns a result every time. Nonetheless, even after checking all potential errors it still says that result.data could be undefined or the object.

Is this intentional @TheEdoRan?