Fix public API errors

[alecmev]

Fix #776
Fix #659

The getDotPath test seems alright, but I'm not sure if the lazy one actually hits the changes, it's a lot of layers to unravel. Can you investigate, please? Also, what's the reason these types were recursive in the first place? Don't mean to pull a Chesterton’s Fence here.

[fabian-hiller]

Thank you for your contribution. It seems to me that your changes fix the "problem" but also introduce new bugs. Does every LazySchema and NonNullableSchema now result in a question mark when used in an object? If so, this leads to wrong types.

[alecmev]

Can you add a failing test to this PR, please?

[fabian-hiller]

key6 and key7 should be undefined but without a question mark. Since every LazySchema and NonNullableSchema now matches QuestionMarkSchema, a question mark is added.

[alecmev]

Thank you! Pushed a fix. I'll be honest, I still don't know how to work around the recursion issue, so this is pure brute-force, but it seems to work. Any potential problems?

[fabian-hiller]

I think you fixed it for exactly one layer, but if we nest multiple lazy or nonNullable schemas it still breaks. That's why I think the recursive implementation is necessary for it to work properly.

[fabian-hiller]

The only and probably simplest way to solve this problem is to define the rule that object only adds a question mark if optional or nullish is the outermost schema of an entry. This way we can remove the recursive implementation.

// With question mark
v.object({ key: v.optional(v.lazy(() => v.string())) }); // { key?: string | undefined } 

// Without question mark
v.object({ key: v.lazy(() => v.optional(v.string())) }); // { key: string | undefined } 

[fabian-hiller]

This would introduce a breaking change, but since we are still v0, this is ok. I like that this rule simplifies the implementation and makes the library usable for more users. But I don't know if there are any major drawbacks yet.

[alecmev]

Tried every possible hack that I could come up with, no luck. Can't dedicate more time to this, sorry. How about this:

• Keep the new tests, for the future.
• Drop the QuestionMarkSchema changes.
• Replace the ignores with /** @ts-expect-error */ for now, don't have to commit.
• Report this in the TypeScript repo, maybe they'll help.

I wouldn't change the API, doesn't feel right to remove something that ultimately works.

[fabian-hiller]

Tried every possible hack that I could come up with, no luck. Can't dedicate more time to this, sorry.

Don't worry about it. I appreciate every contribution! So thank you for the time you have already invested in Valibot. 🙏

I wouldn't change the API, doesn't feel right to remove something that ultimately works.

I see what you mean, but someone could also argue why there is a question mark when optional is nested very deep. It actually makes sense to allow users to control this behavior by intentionally using optional and nullish as the outermost schema within object.

I think that Valibot could become a big and important project in the long run. Therefore, it would be nice to solve the TS error problem and introduce clear rules for such behavior.

[alecmev]

Thank you for caring 😉

Re the API change - what you say makes sense to me, but I think I shouldn't participate in this decision, as I haven't even used .lazy or .nonNullable.

[fabian-hiller]

Another thing you could try is to install Valibot via JSR, as it should ship our source code including the comments: https://jsr.io/@valibot/valibot

[fabian-hiller]

I fixed the TS errors. Thanks for your cooperation!

[alecmev]

Great, thanks! 👍