Skip to content

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

Merged
Brian-McBride merged 15 commits into
anatine:mainfrom
A5rocks:openapi-3.1
Mar 20, 2024
Merged

More complete OpenAPI 3.1 support in zod-openapi.#173
Brian-McBride merged 15 commits into
anatine:mainfrom
A5rocks:openapi-3.1

Conversation

@A5rocks

@A5rocks A5rocks commented Nov 23, 2023

Copy link
Copy Markdown
Contributor

Split off from #130.

@nx-cloud

nx-cloud Bot commented Nov 23, 2023

Copy link
Copy Markdown

☁️ 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
Copy Markdown
Contributor

@A5rocks Can you resolve the conflicts here?

@nx-cloud

nx-cloud Bot commented Jan 18, 2024

Copy link
Copy Markdown

☁️ 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

A5rocks commented Jan 18, 2024

Copy link
Copy Markdown
Contributor Author

Alright, conflicts should now be fixed!

Comment thread packages/zod-nestjs/src/lib/create-zod-dto.ts Outdated
const schemaObjectWithRequiredField = {
const schemaObjectWithFixedFields = {
...schemaObject,
type: schemaObject.type ? schemaObject.type[0] : schemaObject.type,

@erkstruwe erkstruwe Jan 18, 2024

Copy link
Copy Markdown
Contributor

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.

@A5rocks A5rocks Jan 19, 2024

Copy link
Copy Markdown
Contributor Author

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

RobbyUitbeijerse commented Feb 12, 2024

Copy link
Copy Markdown

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
Copy Markdown

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

@nikelborm

Copy link
Copy Markdown

@lewinskimaciej Probably resolve merge conflicts?

@Brian-McBride

Copy link
Copy Markdown
Contributor

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

@A5rocks

A5rocks commented Mar 19, 2024

Copy link
Copy Markdown
Contributor Author

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

@nikelborm

Copy link
Copy Markdown

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

A5rocks commented Mar 20, 2024

Copy link
Copy Markdown
Contributor Author

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

@nikelborm

nikelborm commented Mar 20, 2024

Copy link
Copy Markdown

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
@nikelborm

Copy link
Copy Markdown

Yay! 🎉

@lewinskimaciej

lewinskimaciej commented Mar 21, 2024

Copy link
Copy Markdown

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