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

More complete OpenAPI 3.1 support in zod-openapi. #173

Merged
merged 15 commits into from
Mar 20, 2024

Conversation

A5rocks
Copy link
Contributor

@A5rocks A5rocks commented Nov 23, 2023

Split off from #130.

Copy link

nx-cloud bot commented Nov 23, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 89578a7. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@Brian-McBride
Copy link
Contributor

@A5rocks Can you resolve the conflicts here?

Copy link

nx-cloud bot commented Jan 18, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 6fdba79. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@A5rocks
Copy link
Contributor Author

A5rocks commented Jan 18, 2024

Alright, conflicts should now be fixed!

...schemaObject,
type: schemaObject.type ? schemaObject.type[0] : schemaObject.type,
Copy link
Contributor

@erkstruwe erkstruwe Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove 'null' from type array then. Besides, this doesn't work with string values (rather than arrays) which TypeScript allows here. See #182.

Copy link
Contributor Author

@A5rocks A5rocks Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my changes make this work, but there's a case (or two) which I'm not sure about: type: [] or type: ["null"]. What do you think should happen then?

As of latest commit those should lead to undefined type which is... Idk, should this error instead?

@RobbyUitbeijerse
Copy link

RobbyUitbeijerse commented Feb 12, 2024

hey there! @Brian-McBride / @erkstruwe , this looks rather ready and exactly what our team is looking for. Any chance this could be merged & released? We would love the 3.1 support to reference other Zod schemas, like generated Zod enums coming from https://www.npmjs.com/package/ts-to-zod , in manually created schemas. That currently doesn't work due to the missing $ref/3.1 support, but would greatly improve the output of the DTOs

@lewinskimaciej
Copy link

Up, anything we can do to get this pushed out?

@nikelborm
Copy link

@lewinskimaciej Probably resolve merge conflicts?

@Brian-McBride
Copy link
Contributor

Yes, please resolve then the CI/CD can deploy.

@A5rocks
Copy link
Contributor Author

A5rocks commented Mar 19, 2024

Hmm, CI failure doesn't look like my fault. Tests seem to pass, at least.

@nikelborm
Copy link

Hmm, CI failure doesn't look like my fault. Tests seem to pass, at least.

There is one failing: https://github.com/anatine/zod-plugins/actions/runs/8350755969/job/22857844183?pr=173#step:7:96

@A5rocks
Copy link
Contributor Author

A5rocks commented Mar 20, 2024

Oops, I didn't notice that! Thought the red highlighted line was during setup so only looked after that...

@nikelborm
Copy link

nikelborm commented Mar 20, 2024

I think repo owners or maintainers have to configure problem-matchers for this github action to highlight failed test cases

@Brian-McBride Brian-McBride merged commit 79fbea8 into anatine:main Mar 20, 2024
2 checks passed
@nikelborm
Copy link

Yay! 🎉

@lewinskimaciej
Copy link

lewinskimaciej commented Mar 21, 2024

Nice, thank you guys! ❤️

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.

6 participants