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

WIP: force one of flag #180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmmoret
Copy link

@fmmoret fmmoret commented Oct 21, 2023

Hey, I am working with an API that I cannot add discriminators to, but downstream generators can't consume AnyOf properly in some cases.

Here are some similar threads:
#162
#163

This PR is a proposal for an interface to let a user opt into a potentially unsafe representation of their schema.

If the direction is approved, I can add testing / ensure correctness of my implementation -- I have not manually checked it yet for correctness, but want feedback on the interface & approach.

@fmmoret
Copy link
Author

fmmoret commented Oct 21, 2023

cc @AGalabov

@AGalabov
Copy link
Collaborator

AGalabov commented Nov 2, 2023

@fmmoret Thank you for opening a PR and sorry for taking long on this. I think I do agree that we can add such an option (as opposed to the previous implementation in #163).

A couple of pointers:

  1. I think it makes sense to be consistent across the document => I would rather move the configuration at constructor level instead of through the .openapi method. I simply cannot envision why one would want to have two ZodUnion schemas with different signature within a single document.
  2. We tend to add tests for all features/bugs that we handle. Please add some for this.
  3. I see some checks are failing - please take a look at them

I'd also cc: @georgyangelov to share his opinion on the matter.

@georgyangelov
Copy link
Collaborator

@fmmoret thank you for the contribution.

@AGalabov I'm a bit on the fence for constructor parameter vs .openapi. On one hand the .openapi approach gives more control - you can use oneOf for some of the types (I also can't think of a reason to want that but that doesn't mean there isn't any). On the other hand - that way would mean going through all of your types and adding that to every single one where a union may be declared. The latter is I believe more of an issue so I support the global switch in the geenrator object approach - that would mean users won't forget to add the flag when the schemas change.

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

Successfully merging this pull request may close these issues.

None yet

3 participants