Skip to content
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

[zod-nestjs] inconsistent usage of OAS 3.0.0 vs 3.1.0 (createZodDto doesn't support specifying OAS version) #221

Open
alex-statsig opened this issue Sep 17, 2024 · 3 comments

Comments

@alex-statsig
Copy link

Currently, createZodDto uses generateSchema with no argument for the version, which generates a v3.1 spec. It then manually modifies the generated object to attempt to convert it back to v3.0 for nestjs/swagger's sake. This includes things like fixing "exclusiveMinimum" being a number (not boolean), or types being an array, with 'null' as a type rather than using "nullable".

This is inconsistent with patchNestjsSwagger, which supports passing in v3.0 or v3.1 (defaulting to v3.0, and forwarding that to generateSchema), but fails to properly convert several things to v3.0 (such as nullable for v3.0, as mentioned in this issue, or exclusiveMinimum).

These issues combined generate a spec where half will be 3.0.0 (manually downgraded from 3.1.0 in createZodDto) and the other half will either be valid 3.1.0 (from patchNestjsSwagger overridden to v3.1.0) or corrupt 3.0.0 (due to issues like #211).

Overall it seems like this library is in a broken state, supporting v3.0 half-way and v3.1 half-way, but neither the entire way unfortunately, due to competing interests for supporting one or the other (ex. #173 and #206); I'd love to contribute fixes for these things, but it's unclear the best path to fixing the library (is the goal to support v3.1 first-class and manually convert from that to 3.0 such as in createZodDto, or to actually support generating both 3.0 and 3.1 directly such as in patchNestjsSwagger?). Is there an active owner/maintainer of the repo (@Brian-McBride / @A5rocks) to get thoughts on the best approach to fix these issues from?

@alex-statsig
Copy link
Author

I found some more discussion on this in #181; it seems like the issues are:

  1. patchNestjsSwagger does not apply the same downgrading logic as createZodDto (suggested in [zod-openapi] Null handling in NestJS #181, added in [zod-nestjs] Convert OpenAPI 3.1 to OpenAPI 3.0 for consumption by @nestjs/swagger #182 and [zod-nestjs] Recursively convert to OpenAPI 3.0 specification #185). I imagine we could extract the logic from SchemaHolderClass.convertSchemaObject for this?
  2. There is no way to opt-out of this downgrade to 3.0, even though @nestjs/swagger seems to work with 3.1 if you override their version (and I imagine will eventually get first-class support). This is maybe the most straight-forward fix.
  3. Meanwhile, there is a totally separate and broken way to support v3.0 added in Bring back support for Open API v3.0 #206, which does not properly handle nullable or exclusiveMinimum (I think it mostly just enables not using arrays for the "type" field). This is competing with (1) for how to handle OAS 3.0.0, making it really hard to develop in this package

@A5rocks
Copy link
Contributor

A5rocks commented Sep 17, 2024

#130 (comment) was the closest I've found for an answer. Inferring from the answer, I assume the goal is 3.1 first-class. But I can't imagine it's that much of an issue to support 3.0 at the same time -- the reason I made #173 was because I found working with hybrid 3.0-and-3.1 without any sort of version selection annoying, but that's only annoying due to the lack of version selection not due to the hybrid nature.

@alex-statsig alex-statsig changed the title [zod-nestjs] inconsistent usage of OAS 3.0.0 vs 3.1.1 (createZodDto doesn't support specifying OAS version) [zod-nestjs] inconsistent usage of OAS 3.0.0 vs 3.1.0 (createZodDto doesn't support specifying OAS version) Sep 17, 2024
@matt-statsig
Copy link

@Brian-McBride @A5rocks do you think any maintainer will be able to investigate this one soon? We're wondering because our team really wants to generate and open API spec that is complete and we depend on this package in our codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants