Skip to content

Conversation

@sacrosanctic
Copy link
Contributor

This is a PR carried over from sinclairzx81/typebox-workbench#17

3 things to consider

  • should i use Types.Exclude(value, Types.Undefined()) or Types.TypeGuard.IsNever(Types.Extract(value, Types.Undefined())) to determine whether a schema contains undefined.
  • is it possible to exclude multiple types
  • ive left out exactlOptional as I did not know how to implement the option. If you can point me in the right direction. I can open a PR.

@sinclairzx81
Copy link
Owner

@sacrosanctic Hi, sorry for the delay (have been a bit stretched lately)

Feedback on the update / questions below....


should i use Types.Exclude(value, Types.Undefined()) or Types.TypeGuard.IsNever(Types.Extract(value, Types.Undefined())) to determine whether a schema contains undefined.

I'm not sure I follow the logic here. There shouldn't be a need to extract never (afaik) from the properties.

Workbench Link

interface Test {
  x?: number // expect x: v.optional(v.number()) - ??
}

I might be missing something here. Is Valibot doing something special with respect to baseline optionality?


is it possible to exclude multiple types

TypeBox does support excluding multiple types via Union

const Numbers  = Type.Union([
  Type.Literal(1), Type.Literal(2),
  Type.Literal(3), Type.Literal(4),
])

const Odd = Type.Union([
  Type.Literal(1),
  Type.Literal(3),
])

const Even = Type.Exclude(Numbers, Odd)

// const Even: TUnion<[TLiteral<2>, TLiteral<4>]>

... but not sure we need to in this instance. I think we can trust the Model to be correct just map 1-1 without excluding anything, but again let me know the rational for the optionality handling (I haven't reviewed Valibot 1.0 in depth, so I might be missing something)


ive left out exactlOptional as I did not know how to implement the option. If you can point me in the right direction. I can open a PR.

No problem. I can take a look at this later (it needs top level configurations written for it so couldn't be integrated into the workbench until those are ready). I will say, the exactOptional() modifier type in Valibot may turn out to be a design problem in the long run.

TypeBox also supports this option via a global configuration.

import { TypeSystemPolicy } from '@sinclair/typebox/system'

// Disallow undefined values for optional properties (default is false)
//
// const A: { x?: number } = { x: undefined } - disallowed when enabled

TypeSystemPolicy.ExactOptionalPropertyTypes = true

Setting this policy to true only updates the validation check logic ... it doesn't contribute to inference at all....

The issue is that if a user decides to update to exactOptionalPropertyTypes in tsconfig.json, the user must also remember to update their each v.optional() to v.exactOptional(). Granted global policy configurations for are not ideal (I can understand why Valibot chose not to), but the policy for assertion (both TS and Runtime) is global .... so implementing a corresponding global configuration is mostly unavoidable if the library implementation seeks to remain resilient to tsconfig.json changes (also having two modifiers describing the same behavior is going to hurt in the long term (TypeBox has tripped over similar issues over the years, and is largely why it's opted for global configurations for these kinds of things).

I think ignoring the exactOptional() type is the right thing to do for now, but may drop it in as a configuration option later.


Again, sorry for the delay. As it stands, the Void, Undefined and Null updates are perfect, just need a bit more insight into the optionality handling before merging through.

Keep me posted
S

@sacrosanctic
Copy link
Contributor Author

Thank you for your feedback, I will forward it to the valibot maintainer.

After taking a closer look, I'm not sure it makes sense to remove exactOptional, If the user have exactOptionalPropertyTypes = false, then Typescript will simply resolve to the same type (see Key2 and Key4 in the playground) Valibot Playground

In those cases, the schema will be more specific than the TS type, which should be fine.

// input
const schema = v.object({
  key1: v.string(),
  key2: v.optional(v.string()),
  key3: v.nullish(v.string()),
  key4: v.exactOptional(v.string()),
  key5: v.nullable(v.string()),
  key6: v.undefinedable(v.string())
})
// output
type schema = {
    key1: string;
    key2?: string | undefined;
    key3?: string | null | undefined;
    key4?: string | undefined;
    key5: string | null;
    key6: string | undefined;
}

Is there a way to write test cases? It'll be easier for me to validate the correctness of the code.

@sinclairzx81
Copy link
Owner

sinclairzx81 commented Mar 27, 2025

@sacrosanctic Hiya

See below for additional feedback


Optional

After taking a closer look, I'm not sure it makes sense to remove exactOptional, If the user have exactOptionalPropertyTypes = false, then Typescript will simply resolve to the same type (see Key2 and Key4 in the playground)

Yeah, there is a semantic difference between default and exact. But even with that difference I am still keen to use optional() over exactOptional() as this is the TypeScript default (this doesn't rule out implementing exact via configuration however, something I can take a look at after this PR goes through)


Higher Order / Alias Types

Regarding nullable(), undefinable(), and nullish(), let's drop these and stick with the original Object property translation. I’d prefer to avoid having the codegen emit library-specific higher order alias types. This is mostly to keep the generated Valibot code aligned to known TypeScript constructs

On this specifically, Fabian might be able to shave off additional bytes by removing these aliases and encouraging users to adopt generic runtime types. The following code implements these aliases (external) to Valibot via function generics.

Valibot Generics

import v from 'valibot'

// TypeScript
type Nullish<Type> = Type | null | undefined
type Nullable<Type> = Type | null
type Undefinedable<Type> = Type | undefined

// Valibot - Let users implement higher order types themselves
const nullish = <Type extends  v.BaseSchema<any, any, any>>(type: Type) =>
  v.union([type, v.null(), v.undefined()])
const nullable = <Type extends  v.BaseSchema<any, any, any>>(type: Type) =>
  v.union([type, v.null()])
const undefinable = <Type extends  v.BaseSchema<any, any, any>>(type: Type) =>
  v.union([type, v.undefined()])


// Test
const A = nullish(v.string())
const B = nullable(v.string())
const C = undefinable(v.string())

All this said, for code translation, we have to generate the Union form, not the higher order form.


Reference Runtime Translation

As a reference, I actually manage another translation project (typemap) which performs a similar type translation at runtime. Ideally we need the codegen project generating the same types as typemap, so have a quick look to see how it handles those optionals, nullables and undefinables.

Reference Translation Link Here

import { Valibot, Syntax } from '@sinclair/typemap'

// hover schema for runtime translated valibot type
const schema = Valibot(`{
    key1: string;
    key2?: string | undefined;
    key3?: string | null | undefined;
    key4?: string | undefined;
    key5: string | null;
    key6: string | undefined;
}`)

// hover schema for reverse translation
const syntax = Syntax(schema)

Testing

Is there a way to write test cases? It'll be easier for me to validate the correctness of the code.

Unfortunately no :( Historically I either test translations from the Workbench, or setup a library project and either copy or paste the translation into that project (or write a quick script to do it). The base model (TypeBox) is the source of truth for translation (which is heavily tested elsewhere), so as long as you trust the structures from TypeBox, and there is an equivalent type to translate to, things should be ok (it's another reason to keep the translations simple)

I am open to ideas on better ways to test translations though, so if you have any thoughts, let me know.


There's a bit of depth to type translation that's a bit difficult to communicate in a PR thread (particularly as it relates to other projects). But for now let's revert the Object translation function (I think the previous implementation is going to be compatible with Valibot 1.0). I am very happy with the updated emitters for null, undefined and void though, so let's keep those ones.

Once this PR goes through, I can take a look at providing an option to generate strictOptional().

Cheers!
S

Copy link
Contributor Author

@sacrosanctic sacrosanctic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@sinclairzx81
Copy link
Owner

@sacrosanctic Nice work.

Will merge this through and add the configuration for exactOptional. Will publish updates to the Workbench shortly after.

Will notify when everything is published through.

Cheers!

@sinclairzx81 sinclairzx81 merged commit 9faeeea into sinclairzx81:main Mar 27, 2025
3 checks passed
@sinclairzx81
Copy link
Owner

@sacrosanctic Updates published to

  • @sinclair/typebox-codegen@0.11.0
  • Workbench (make sure you clear you cache)

I also updated the Valibot transform to accept a exactOptionalPropertyTypes setting. This isn't enabled on the Workbench as discussed, but if you are programmatically using the code generators, you can generate exactOptional() with the following.

const model = Codegen.TypeScriptToModel.Generate(Code)

const code = Codegen.ModelToValibot.Generate(model, {
  exactOptionalPropertyTypes: true
})

Thanks again!
All the best
S

@sacrosanctic sacrosanctic deleted the tight-crow branch March 27, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants