-
Notifications
You must be signed in to change notification settings - Fork 29
valibot@1.0.0 #55
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
valibot@1.0.0 #55
Conversation
|
@sacrosanctic Hi, sorry for the delay (have been a bit stretched lately) Feedback on the update / questions below....
I'm not sure I follow the logic here. There shouldn't be a need to extract 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?
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)
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 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 = trueSetting this policy to The issue is that if a user decides to update to I think ignoring the Again, sorry for the delay. As it stands, the Keep me posted |
|
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 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. |
|
@sacrosanctic Hiya See below for additional feedback Optional
Yeah, there is a semantic difference between default and exact. But even with that difference I am still keen to use Higher Order / Alias TypesRegarding 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. 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 TranslationAs 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
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 Once this PR goes through, I can take a look at providing an option to generate Cheers! |
sacrosanctic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
@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! |
|
@sacrosanctic Updates published to
I also updated the Valibot transform to accept a const model = Codegen.TypeScriptToModel.Generate(Code)
const code = Codegen.ModelToValibot.Generate(model, {
exactOptionalPropertyTypes: true
})Thanks again! |
This is a PR carried over from sinclairzx81/typebox-workbench#17
3 things to consider
Types.Exclude(value, Types.Undefined())orTypes.TypeGuard.IsNever(Types.Extract(value, Types.Undefined()))to determine whether a schema contains undefined.exactlOptionalas I did not know how to implement the option. If you can point me in the right direction. I can open a PR.