-
-
Notifications
You must be signed in to change notification settings - Fork 244
fix(to-json-schema): translate v.file()
schema
#1119
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
base: main
Are you sure you want to change the base?
Conversation
A `v.file()` can/should be translated to the JSON schema: ```yml type: string format: binary ``` See https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/ for OpenAPI support. I initally [thought this was an issue](rhinobase/hono-openapi#66 (comment)) with `hono-openapi`'s translation, but tracked it to here instead.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR addresses the issue of translating a v.file() schema to a JSON schema that complies with OpenAPI by mapping it to a string type with binary format. The key changes include:
- Adding a case in convertSchema for handling v.file() and mapping it to type 'string' with format 'binary'.
- Updating the unit tests in convertSchema.test.ts to ensure the new file schema conversion works as expected.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/to-json-schema/src/convertSchema.ts | Added new case to translate v.file() to a JSON schema with type string and format binary |
packages/to-json-schema/src/convertSchema.test.ts | Added a test case to validate the new file schema conversion |
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
Thanks for creating this PR! I am not sure if we should convert the const Schema = v.pipe(v.string(), v.binary()); |
commit: |
The OpenAPI spec says that files need to be string/binary. Because that’s what they are. https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/ The translation is going from one representation to another. The rest of the ecosystem on top of valibot is already using v.file() for formdata validation, which works after req.formData() parsing…but that needs to be recorded and described in OpenAPI format too. |
Would you also convert |
Out of curiosity, what are the disadvantages of defining a binary string as |
When forms are uploaded, they are encoded as binary strings. Thru HTTP headers, those strings are known to be 1) sliced by a boundary 2) some are indicated as being files 3) of a certain mime type On the JS server, But before getting to the server, OpenAPI clients need to know that whether or not file(s) can be sent. In OpenAPI definitions (thru JSON schema) this is represented as However, there is now a draft-9 spec change that incorporates the (still optional) If I were you, I would do this: v.file();
//-> {
//-> "type": "string",
//-> "format": "binary",
//-> }
// ImageSchema
v.pipe(
v.file('Please select an image file.'),
v.mimeType(['image/jpeg', 'image/png'], 'Please select a JPEG or PNG file.'),
v.maxSize(1024 * 1024 * 10, 'Please select a file smaller than 10 MB.')
);
//-> {
//-> "oneOf": [{
//-> "type": "string",
//-> "format": "binary",
//-> "contentMediaType": "image/jpeg",
//-> "maxLength": 1024 * 1024 * 10
//-> }, {
//-> "type": "string",
//-> "format": "binary",
//-> "contentMediaType": "image/png",
//-> "maxLength": 1024 * 1024 * 10
//-> }
//-> } You can't force/assume a
In the example above, once user
Blobs can't be uploaded. It's a JS specific API and isn't HTTP transferrable. At best, JSON schema could use a
This could perhaps be an option, but I wouldn't. Like |
Thank you for taking the time to explain it so clearly! I really appreciate it! I don't have a lot of time right now, but I will try to review and merge within the next few days and publish a new version. |
JSON Schema does not have |
You can use anything in a format. There are common formats that have been added thru general agreement, but the contents of |
Does |
@fabian-hiller sorry I don’t understand the question |
Sorry, my bad. I fixed my previous comment. |
Ah. No, you can’t add any more than what developer specifies. I’ll point to the above example snippet again. On its own, you can only translate v.file() to string/format=binary |
I am still not sure what to do. Both Zod v4 and ArkType also throw an error when converting a |
I just added 3 new configs with |
A
v.file()
can/should be translated to the JSON schema:See https://swagger.io/docs/specification/v3_0/describing-request-body/file-upload/ for OpenAPI support.
I initally thought this was an issue with
hono-openapi
's translation, but tracked it to here instead.