search

elierotenberg / fastify-zod

Zod integration with Fastify
MIT License
212 stars 19 forks source link

mergeRefs by default #5

Closed capaj closed 2 years ago

capaj commented 2 years ago

seems like mergeRefs would be a sensible default. Why is it opt-in and not opt-out?

elierotenberg commented 2 years ago

Hi!

Thanks for your input. Not only do I agree, but I spent most of the last few days digging through the ecosystem to try and come up with a better solution.

First, upstream zod-to-json-schema already does some merging, but only when using a single top-level object, which is not how fastify-zod currently works. So I tried to refactor to benefit from this.

The way zod-to-json-schema handles this is JSON pointers with deep refs (e.g. $ref: '#/components/schemas/my-schema/properties/property1'). I hit a bug in fastify-swagger with a PR pending when using JSON pointers in openapi mode.

It turns out that even with this fix, downstream openapitools-generator has no support for this either...

So - I rolled up my sleeves and implemented JSON pointer flattening, which required significant refactoring. While doing that, I realized we could expose a simpler API where there is no need to duplicate explicit typing.

The result is 1.0.x, which should be backwards compatible. Very happy to have your feedback on this.

elierotenberg commented 2 years ago

I've finally decided to take a very different approach for 1.0.x. Trying to satisfy both fastify and openapitools-generator simultaneously proved to be very challenging if not impossible. So the new approach is to avoid complex transformations in buildJsonSchemas, but apply much more drastic ones in a separate SpecTransformer added in 1.0.x. The SpecTransformer transforms include rewriting $refs in schemas, extracting properties as new schemas, and schema merging. Transforms are much safer and errors (if any) should raise much more helpful messages.

Please let me know your opinion.

elierotenberg commented 2 years ago

I've finally decided to take a very different approach for 1.0.x. Trying to satisfy both fastify and openapitools-generator simultaneously proved to be very challenging if not impossible. So the new approach is to avoid complex transformations in buildJsonSchemas, but apply much more drastic ones in a separate SpecTransformer added in 1.0.x. The SpecTransformer transforms include rewriting $refs in schemas, extracting properties as new schemas, and schema merging. Transforms are much safer and errors (if any) should raise much more helpful messages.

Please let me know your opinion.

elierotenberg commented 2 years ago

1.0.0 is up :)