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(validation-errors): type inference for branded, nullable, optional and array types #284

Closed aleclofabbro closed 3 weeks ago

aleclofabbro commented 3 weeks ago

returnValidationErrors() typechecking fails when a property is a primitive-object-intersection type (typical case of branded type).

returnValidationErrors() refers to type SchemaErrors<S> for typings the purpose of type SchemaErrors<S> as is:

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

is to map S properties conditionally: if property S[K] is object | null | undefined maps to Prettify<VEList & SchemaErrors<S[K]>> else (property S[K] is primitive) maps to VEList

the issue arises when property S[K] 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 })

type SchemaErrors<S> mapping condition considers branded-primitive-types as object, mapping them incorrectly, and finally leading to returnValidationErrors() type errors

Proposed changes

the issue is resolved by simply reversing the condition:

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

this way any primitive - even when intersected - is eagerly catched and properly managed, keeping logic unaltered

Related issue(s) or discussion(s)

This is an amend of a previous (closed) PR https://github.com/TheEdoRan/next-safe-action/pull/277

re #


vercel[bot] commented 3 weeks 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 Oct 22, 2024 2:07pm
next-safe-action-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 22, 2024 2:07pm
vercel[bot] commented 3 weeks 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 3 weeks ago

@aleclofabbro updated code to fix inference for optionals, nullables and arrays too. Please check it out, lgtm so I think it can be merged.

aleclofabbro commented 3 weeks ago

@TheEdoRan are you sure | null | undefined | any[] should be added to the condition ? the original puts together objects, null, and undefined ( S[K] extends object | null | undefined ) having null and undefined to be pushed to the Prettify<VEList & SchemaErrors<S[K]>> branch , the same for any[], being it an object optionals should not be an issue, as all properties are mapped to be optional, and the S[K] extends condition checks for the defined prop type, even for optional properties makes sense ?

TheEdoRan commented 3 weeks ago

@aleclofabbro yep, while I was working on this I found out that arrays, nullables and optionals weren't typed as expected. In fact, if I remove null | undefined | any[] from the NotObject type, having a schema like this one below, the resulting type of ValidationErrors isn't correct:

image

If I instead include those additional types in the NotObject type, the resulting type of ValidationErrors is correct:

image

aleclofabbro commented 3 weeks ago

ha! so that was an existent bug you spotted out while checking my PR ? if so, that's simply great, pow-pow ! lgtm too , any action from me ?

TheEdoRan commented 3 weeks ago

@aleclofabbro Yeah! haha. No actions required from you, gonna merge this soon, thank you for your contribution!

github-actions[bot] commented 3 weeks ago

:tada: This PR is included in version 7.9.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: