-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add Schema Reflection API #211
Changes from 12 commits
a0656aa
e59e541
9807fe0
489d974
f776d8c
0633512
3eab476
cedef6b
2104a07
e78a150
3d51e70
1cf3af4
9ad82d6
da3ca63
775d539
5838643
403ba7e
16fd0f1
a90ac7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,11 @@ import { | |
* Bigint schema type. | ||
*/ | ||
export type BigintSchema<TOutput = bigint> = BaseSchema<bigint, TOutput> & { | ||
schema: 'bigint'; | ||
kind: 'bigint'; | ||
/** | ||
* Validation and transformation pipe. | ||
*/ | ||
pipe: Pipe<bigint>; | ||
}; | ||
|
||
/** | ||
|
@@ -36,28 +40,13 @@ export function bigint( | |
arg2?: Pipe<bigint> | ||
): BigintSchema { | ||
// Get error and pipe argument | ||
const [error, pipe] = getDefaultArgs(arg1, arg2); | ||
const [error, pipe = []] = getDefaultArgs(arg1, arg2); | ||
|
||
// Create and return bigint schema | ||
return { | ||
/** | ||
* The schema type. | ||
*/ | ||
schema: 'bigint', | ||
|
||
/** | ||
* Whether it's async. | ||
*/ | ||
kind: 'bigint', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think It's also what I've seen commonly used to differentiate similar types in discriminated unions, which is basically what we have here to refer to all schemas/validations/transforms that extend a common base class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If anything, this would be better read as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand the JSON argument, but would not consider it for Valibot as it seems too irrelevant at the moment. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's a good argument! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have been reading about the difference between |
||
async: false, | ||
|
||
/** | ||
* Parses unknown input based on its schema. | ||
* | ||
* @param input The input to be parsed. | ||
* @param info The parse info. | ||
* | ||
* @returns The parsed output. | ||
*/ | ||
pipe, | ||
_parse(input, info) { | ||
// Check type of input | ||
if (typeof input !== 'bigint') { | ||
|
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.
For types, I don't think these inline comments are necessary.
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.
As a result of your comment, I have changed my mind on this. Thank you for this improvement. However, I would prefer to add a blank line before a comment that starts with
/**
for a better overview.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.
I'll make a pass over everything tonight to try and clean up the formatting.