Upgrade and migrate drizzle-valibot to v0.31.0 #2481
Conversation
|
@fabian-hiller pnpm-lock is 9.0 now in main so you can update it |
|
Done |
| : PicklistSchema<TColumn['enumValues']> | ||
| // TODO: Can we somehow check `.length` to know if it's a string schema with or without a max length check? | ||
| ? Equal<TColumn['enumValues'], [string, ...string[]]> extends true ? StringSchema<undefined> | SchemaWithPipe<[StringSchema<undefined>, MaxLengthAction<string, number, undefined>]> | ||
| : PicklistSchema<TColumn['enumValues'], undefined> |
There was a problem hiding this comment.
Is adding 'readonly' to these lines necessary or is actual meant to be intentionally loose?
Line 102
- : PicklistSchema<TColumn['enumValues'], undefined>
+ : PicklistSchema<Readonly<TColumn['enumValues']>, undefined>Line 135
- [K in keyof TColumns & string]: MaybeOptional<
+ readonly [K in keyof TColumns & string]: MaybeOptional<Line 151
- [K in keyof TTable['_']['columns']]: MaybeOptional<
+ readonly [K in keyof TTable['_']['columns']]: MaybeOptional<Before:
After:
There was a problem hiding this comment.
I think readonly is not necessary and is only added by Valibot because we use TypeScript's as const feature for specific generic types.
There was a problem hiding this comment.
Thanks for clarifying
|
@drizzle-team feel free to take over this PR and reach out if you have any questions. I am happy to help! |
|
What steps would be needed to finish this off from the drizzle-valibot perspective? :) |
|
Review my changes and answer or fix my |
| : TColumn extends { enumValues: [string, ...string[]] } | ||
| ? Equal<TColumn['enumValues'], [string, ...string[]]> extends true ? StringSchema | ||
| : PicklistSchema<TColumn['enumValues']> | ||
| // TODO: Can we somehow check `.length` to know if it's a string schema with or without a max length check? |
There was a problem hiding this comment.
What do you want to get here exacly?
In theory the following should work, but there is a chance that typescript no longer has the exact information of T["length"] and it has just become number.
type MaybeLength<T extends { length?: number }> =
T["length"] extends 0 | undefined ? NoLengthType<0> : LengthType<T["length"]>There was a problem hiding this comment.
I was just trying to be more specific about the returned type. I think the Drizzle core team knows how to proceed and are free to modify the code as I am busy with other things.
Fix `mapColumnToSchema`
|
@dankochetov can you review this PR? |
|
any update on this? 🙏🏼 |
|
Andrew from the Drizzle team told me that they will be reviewing and finalizing it soon. |
|
Is this PR at least in an state that we can patch Drizzle to use it? |
|
I am not sure, but I think so. |
|
Bump, hopefully this isn't lost forever |
|
I hope so too 🙏 |
|
@drizzle-team, @dankochetov, @emmanuelchucks I've added a new PR that is up to date with main (#2860) to supercede this one |
This PR is not perfect and not finished as I had many questions. I added some
TODO:to the code with more details. I also did not change thepnpm-lock.yamlfile because my version ofpnpmautomatically updates it tolockfileVersion: '9.0'.Hint:
BaseSchemais the type we use internally because it forces us to define all generics. This reduces errors on our end. For end users, Valibot also exports theGenericSchematype, which can be used to simplify the code.