fix(zod): treat additionalProperties keyword

[ixth]

Status

READY

Description

additionalProperties: true isn't treated the right way during schema parsing, which results in runtime error during generation. Since #972 is stalled, decided to dig into it myself.

Related PRs

Previous attempt to fix this:

branch	PR
CxGarcia:fix/zod-additional-properties	link

[ixth]

Also, couldn't help but fix few typos here and there.

[ixth]

Actual fix which I'm not sure about: since additionalProperties: true is just an alias for additionalProperties: {}, I decided to treat it as latter. Should definition be faithful to original scheme?

[soartec-lab]

Is it correct to understand that if it is simply true, it is determined that the additional property does not exist?

[ixth-ind]

As far as I understood from the OpenAPI docs, it's the opposite:

If the dictionary values can be of any type (aka free-form object), use additionalProperties: true:

https://swagger.io/docs/specification/data-models/dictionaries/#free-form

So, additionalProperties: true is equivalent to additionalProperties: {}, which means that any type of property values allowed. Basically, it means it's Record<string, any>. additionalProperties: {} works fine, btw, it's just additionalProperties: true that's broken.

UPD. Oops, used work account. Comment is still relevant, though.

[soartec-lab]

Thanks, I understood.

[melloware]

@TommoLeedsy can you review this also? Its related to other recent zod changes

[soartec-lab]

@ixth
I have asked a simple question, please answer.

[soartec-lab]

Is it correct to understand that if it is simply true, it is determined that the additional property does not exist?

[melloware]

@ixth in this other PR that has been open for a long time: #972 he used zod.passthrough()? not sure if your solution is better than what he was trying to do?

[ixth]

@melloware
Can't tell right away, have to check it more thoroughly, but I assume these two points:

1. additionalProperties: {} works just fine, additionalProperties: true is an alias for it, and must behave the same way, i.e. generate z.record(z.string(), z.any())
2. z.passthrough() is a parsing directive, and it doesn't affect inferred types, so it won't work in cases when I want to use z.infer on generated type. I might add a test case for that.

[melloware]

Awesome thanks for the answers @ixth

[soartec-lab]

@melloware @ixth

Hey, it seemed to me that this response wasn't exactly the same as what you're trying to do with #972.
In other words, it is possible to do both. Here we will modify it so that additionalProperties works, but in #972 we will use zod to make it easier to handle properties, right? What do you think?

[ixth]

In other words, it is possible to do both.

As soon as additionalProperties: true will result in z.record(z.string(), z.any()), there will be no need to add .passthrough(): any fields with string key will be allowed and passed through.

I might not get the whole picture. What cases am I missing?

[soartec-lab]

@ixth
I got it. It looks good. There is no specific case, it's just that I have doubts, so it's okay.

[soartec-lab]

Thank you for your response. I think it's a good change, but I'd like to wait for some comments by #972, so I'll merge it in a few days.

[melloware]

@soartec-lab i am going to merge as this is the correct solution and @ixth pointed out why the previous PR was actually incorrect.

[soartec-lab]

Gotcha!