-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
issue parsing optional number #9
Comments
Thank you for creating this issue. The problem with this issue is that as far as I remember, this library only supports one datatype per field. I will look into it and see if and how we can make it possible to support multiple datatypes. |
I have investigated this case and there is a problem. const formData = new FormData();
formData.append('key1', null);
formData.append('key2', undefined);
formData.append('key3', 123);
formData.append('key4', "123");
formData.append('key5', true);
formData.append('key6', "true"); Results in: [
["key1", "null"],
["key2", "undefined"],
["key3", "123"],
["key4", "123"],
["key5", "true"],
["key6", "true"]
] Therefore, for strings, we cannot distinguish between
This means that for anything that isn't a string, we could try to allow multiple data types by improving data type detection in our source code. But for strings this is not possible. |
Can you send me your |
I changed a few things and released a new version. For non-string types:
Please let me know if this solves your problem. |
Interesting. In my case I'm using SvelteKit with: await request.formData() which would be an object. I have a complex schema for a post draft. Basically I am passing: <input name="category_order" type="number" /> You oddly enough set the value with a J |
Interesting. Not sure if this is expected, but I get this: const draftSchema = object({
category_order: optional(number()),
...
});
const formData = decode(await request.formData(), {
numbers: ['category_order']
});
Everything works when not null. J Edit: I tried this too, didn't work, same error: category_order: nullable(optional(number())), |
You can either change your schema to
Are you sure? Please try it again and send me the error message. |
WIth
To me J |
Ok. Then I would leave the implementation as it is for now and close this issue. |
But it doesn't work with |
The warning with |
Ok, so I put this default value as
This is how I believe this should work. If I have As of now, your changes do not fix this problem, as there is not way to declare an optional string. Technically J |
|
So correct me if I'm wrong, but I would think it should match the current way If you pass an empty value in a text field: <input type="text" name="test" /> It will actually just pass an empty string. All inputs are strings and pass an empty string. We differentiate them by their type. However, you don't do: const schema = object({
test: nullable(string())
...
}); You do: const schema = object({
test: optional(string())
...
}); I would think you would want it congruent no matter what the type is. Plus, as I said earlier, Given that is how strings work, shouldn't numbers work the same way? decode(await formData, {
numbers: ['test']
}); Make the test field check for empty strings, and if so convert them to Unless I am missing something? J |
Why do you consider If you don't like |
I have not tested this, but I thought a disabled input returns It's not what I like, I just figured you would want a number input to behave the same way as a text input when it comes to |
If I have an empty string as a value for a |
I'm not sure what you mean. Those are client functions. I made a quick test repo: If you run this: <form method="POST" enctype="application/x-www-form-urlencoded">
<input type="number" name="number" />
<input type="text" name="text" />
<input type="date" name="date" />
<input type="file" name="file" />
<input type="text" disabled />
<input type="submit" />
</form> You get these results: FormData {
[Symbol(state)]: [
{ name: 'number', value: '' },
{ name: 'text', value: '' },
{ name: 'date', value: '' },
{ name: 'file', value: '' }
]
} Note: Forget about All This is fine, but my whole point is that maybe they should be translated to I'm saying maybe it should be consistent across all types when there is no value passed, so I would think using the const schema = object({
text: optional(string()),
number: nullable(number()),
date: nullable(date()),
file: optional(instance(File))
});
const result = safeParse(schema, decode(data,
{ numbers: ['number'], dates: ['date'], files: ['file'] }
)); J |
Can you explain why |
So on the client side, you get: {
"number": "",
"text": "",
"date": "",
"file": {}
} Optional works as a string because it is really This means both I did not realize this. You should not change this, as I would think it should match the frontend. By default, html forms are not required unless you explicitly add So in reality the only problem is Are you a form validation library or an object validation library? I would think you have to be both since form values would be translated to objects first in most use cases. Another option is to add a separate type like Either way, a
J |
You can use |
Ok, so if Currently J |
Can you send me a code example? |
Sorry for confusion. It does show an error for both (I forgot my code only shows first error). Here is the updated the repo. But basically: const schema = object({
text: string(),
number: number(),
date: date(),
file: instance(File)
}); It will show a null error for I guess my thoughts are that they should all be optional and not show an error in any case, unless Or maybe just J |
Valibot is a schema library implemented closely to TypeScript and |
I don't understand what you're meaning here. A The problem we are talking about is the lack of consistency. I think what you really need is this: const result = safeParse(schema, decode(data,
{ strings: ['text'] }
)); That way an empty string is treated as Also, you should fix J |
That's a great idea! Thank you! I will think about it. |
@fabian-hiller - Did you think about doing this? Thanks! J |
Sorry for not answering. Valibot already takes up all my time.
But what if someone deliberately wants to pass an empty string? |
I think the question is about I realize Valibot handles this now (since this post was created) with You could manually add something like |
Thanks for your feedback. Let's wait for more feedback. I am sure we will find a good solution in the long run. |
How would I parse an optional number?
This does not seem to work when optional, as I get this message:
when checking the error:
From this discussion
J
The text was updated successfully, but these errors were encountered: