TheEdoRan / next-safe-action

Type safe and validated Server Actions in your Next.js project.
https://next-safe-action.dev
MIT License
2.29k stars 35 forks source link

fix(packages/next-safe-action/src/validation-errors.types.ts) #273

Closed aleclofabbro closed 1 month ago

aleclofabbro commented 1 month ago

Proposed changes

returnValidationErrors() typechecking fails when a property is a zod-branded-primitive-type, type SchemaErrors<S> considers a zod-branded-primitive-type as an object instead as a primitive, leading to a typecheck error. Fixed type SchemaErrors<S> definition to detect branded-primitives and behave correctly


vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
next-safe-action-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 22, 2024 4:31pm
vercel[bot] commented 1 month ago

@aleclofabbro is attempting to deploy a commit to the Edoardo Ranghieri's projects Team on Vercel.

A member of the Team first needs to authorize it.

TheEdoRan commented 1 month ago

Thanks for the contribution. By including this change (importing a specific Zod feature), all the other supported validation libraries still work as expected? If not, would it be better to modify the adapter type for Zod, if possible?

https://github.com/TheEdoRan/next-safe-action/blob/197ec513e605c0814ab5a1ec1190f979b483437b/packages/next-safe-action/src/adapters/types.ts#L10-L28

aleclofabbro commented 1 month ago

@TheEdoRan everything seems to work well as expected with this edit I understand your concerns introducing a reference to zod.

Indeed, I realize the issue is quite generic, and not zod-specific: the purpose of the type as is:

type SchemaErrors<S> = {
    [K in keyof S]?: S[K] extends object | null | undefined ? Prettify<VEList & SchemaErrors<S[K]>> : VEList;
} & {};

is to map properties to Prettify<VEList & SchemaErrors<S[K]>> if the prop extends object | null | undefined on the other hand if the prop is a primitive it should be mapped to VEList

the issue arises when a property is a branded primitive type. Branded primitive types are typically an intersection of a primitive with an object holding the brand (e.g. type email = string & { [k in brand_prop_type]: unique symbol }) This pattern is generic and it's not a zod peculiarity

So, thinking twice, I see my fix is a bit contrived, and it can be actually rewritten in a neater way, without adding any additional reference simply reversing the condition:

type SchemaErrors<S> = {
  [K in keyof S]?: S[K] extends number | string | boolean | bigint
    ? VEList
    : Prettify<VEList & SchemaErrors<S[K]>>;
} & {};

this way any primitive - even when intersected - is eagerly catched and properly managed

I'll close this pull request and open a new one