feat: support custom serialization / deserialization (WIP) #220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mnahkies
commented
Jun 8, 2024
| exclude: z | ||
| .union([z.array(z.enum(["repositories"])), z.enum(["repositories"])]) | ||
| .optional() | ||
| .transform((it) => (Array.isArray(it) || it === undefined ? it : [it])), |
There was a problem hiding this comment.
It'd be nice to make this function name-able / reusable
mnahkies
commented
Jun 8, 2024
| @@ -248,6 +248,9 @@ export interface Schema { | |||
| externalDocs?: ExternalDocumentation | undefined | |||
| deprecated?: boolean | undefined | |||
| // xml?: XML | undefined | |||
|
|
|||
| // TODO: not yet supported by type-builder or joi | |||
| "x-alpha-transform"?: string | undefined | |||
There was a problem hiding this comment.
It would probably be more useful to have this be shaped like {fn: string, type: string} such that we can plumb the return type through the type-builder properly
mnahkies
commented
Jun 8, 2024
| schema: string, | ||
| transformation: string | ((it: unknown) => unknown), | ||
| ) { | ||
| // TODO: is it possible to do arbitrary transformations with `joi`? |
There was a problem hiding this comment.
It looks like the way to do this with joi would be to register the transformation as an extension like shown in hapijs/joi#2796 (comment)
This will pretty much require that we make the functions nameable / reusable as we wouldn't want to register an extension for the same thing over and over.
mnahkies
force-pushed
the
mn/217/fix-query-param-arrays
branch
from
2c3d2aa to
87dde06
Compare
- introduce `x-alpha-transform` concept, allowing for a arbitrary transformation function to be applied to a schema - not yet supported by `joi` - not yet supported by `type-builder` - only really useful for this specific case so far - detect query parameters of an array type, and parse as a `T | T[]`, and then transform to a `T[]` for convenience - does not yet support `$ref`d schemas properly relates #217
mnahkies
force-pushed
the
mn/217/fix-query-param-arrays
branch
from
87dde06 to
71e466a
Compare
mnahkies
commented
Jun 21, 2024
| @@ -12,7 +12,7 @@ export type t_CreateUpdateTodoList = { | |||
| } | |||
|
|
|||
| export type t_TodoList = { | |||
| created: string | |||
| created: Date | |||
There was a problem hiding this comment.
This will be problematic without --enable-runtime-response-validation enabled, as by default we don't use runtime parsing in the client templates
mnahkies
commented
Jun 21, 2024
| schema: string, | ||
| transformation: string | ((it: unknown) => unknown), | ||
| ) { | ||
| return [schema, `transform(${transformation.toString()})`] |
There was a problem hiding this comment.
We probably actually need to use a preprocess here to get the desired effect, eg:
> z.preprocess(it => Array.isArray(it) ? it : [it], z.array(z.number())).parse([1,2])
[ 1, 2 ]
> z.preprocess(it => Array.isArray(it) ? it : [it], z.array(z.number())).parse(1)
[ 1 ]
mnahkies
added a commit
that referenced
this pull request
Jul 27, 2024
#220 got a bit off-track with scope creep. this PR takes a similar approach, but for internal use only - detects query parameters with array type schemas - adds an internal annotation to indicate these need preprocessing - applies a `z.preprocess` to coerce an individual value to an array of 1 element, before parsing with the array schema - for `joi` we resort to a pretty hacky wrapping of the schema with a object that does the preprocessing, as the `joi` extension API didn't work as I'd expected it to. will continue experimenting with making this a more generally useful feature for handling things like parsing `date-time` strings to be a `Date`, etc, separately. fixes #217 **Testing Notes** Need to improve automated test coverage still, but manually tested on a running server: - No query params - 1 element - 2 elements - 2 elements, 1 element and it seems to be working correctly. ``` listening on http://127.0.0.1:3000 query { query: {} } query { query: { statuses: [ 'complete' ] } } query { query: { statuses: [ 'complete', 'incomplete' ] } } query { query: { statuses: [ 'complete', 'incomplete' ] } } query { query: { statuses: [ 'complete', 'incomplete' ], tags: [ '123' ] } } ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(#225 solved the bug originally motivating this, but intend to experiment with ways to allow custom serialization / de-serialization in general on this branch)
x-alpha-transformconcept, allowing for a arbitrary transformation function to be applied to a schemajoitype-builderT | T[], and then transform to aT[]for convenience$refd schemas properlyrelates #217