Parser crash for nonOptional / nonNullish wrapped around transformers emit undefined

[NiklasPor]

While building a base type which maps empty strings to undefined, I stumbled about the minLength & maxLength functions crashing, when passed an undefined value which was wrapped in a nonOptional or nonNullish:

import * as v from 'valibot';

const String = v.nonOptional(
  v.pipe(
    v.string(),
    v.transform((val) => val?.trim() || undefined),
  ),
);

const Password = v.pipe(String, v.minLength(6))

const Schema = v.object({
  password: Password
});

const result = v.safeParse(Schema, {
  password: "",
});

console.log(result);

This actually crashes the execution, even though safe Parse is used:

[error]: TypeError: Cannot read properties of undefined (reading 'length')
    at ~validate (https://valibot.dev/assets/Cb45s_t9-index.min.js:1:18263)
    at ~validate (https://valibot.dev/assets/Cb45s_t9-index.min.js:1:64883)
    at ~validate (https://valibot.dev/assets/Cb45s_t9-index.min.js:1:45222)
    at Module.Ae (https://valibot.dev/assets/Cb45s_t9-index.min.js:1:65578)
    at about:srcdoc:152:18

Playground example: https://valibot.dev/playground/?code=JYWwDg9gTgLgBAKjgQwM5wG5wGZQiOAcg2QBtgAjCGQgbgCh6BjCAO1XgGUYphWBzOAF5MAOlZsA8mBjA2ZABT04YsMDABTJSpUZRHXgIUBKADTKdensnbZoIBQpKljwgHyYyAflE9QJuAAfQLgAV1YAEw1sPg0IswsE4wZmNg44AAU0VAB3aAjhVXUtbkN+UzEQPgAZDQEYAAsFADZjY0YWdi4mBo0QZEK9CAoAKw0mGAUAbwswbLyoCIAuTPn8+gBfZI60+CgNVFDSeBE9VGRsDSyoVBKevuQKmZU51Fz8lYAiT-MtlM7UBBSBpRKQIPwFPtDsdtkA

[NiklasPor]

So I think I had a misconception about the nonOptional / nonNullish. I expected it to throw if the output error of the passed schema gives us undefined, but seems that its working in a different way. Here's a simpler example, type intellisense shows us string, but in reality nonOptional passes and still returns us undefined.

import * as v from 'valibot';

const BaseString = v.pipe(
  v.union([v.string(), v.undefined()]),
  v.transform(val => val?.trim() ? val : undefined)
)

const NonOptionalBaseString = v.nonOptional(BaseString)

const result = v.parse(NonOptionalBaseString, "");
// Type intellisense shows us: const result: string

// But result gives us `undefined`
console.log(result);

Playground link

[fabian-hiller]

You hit an edge point. I will investigate what we can do to prevent this. The nonOptional schema only checks the input value, not the output value. The type signature is a known problem: https://github.com/fabian-hiller/valibot/blob/main/library/src/schemas/nonOptional/types.ts#L54-L55

Source code: https://github.com/fabian-hiller/valibot/blob/main/library/src/schemas/nonOptional/nonOptional.ts#L92-L96

[fabian-hiller]

How do you expect nonOptional to work? Should it only return an issue for undefined inputs or also for undefined outputs and therefore return an issue for the transformation to undefined?

[NiklasPor]

Yes, at least that's what I expected – but that not necessarily means it's the right thing todo.

• I think for the actions which are piped after each other it's clear, they get the value from the previous action and then apply their changes.
• For the functions which are called on another schema, like nonOptional it's not so clear. I expected it to "take any schema I pass in and make sure it's not undefined". Regarding whether it operates on inputs or on outputs, I'm largely focused on the final output as I'm doing a lot of parsing.

Hope this helps, for now I use my own helper as a replacement.

[NiklasPor]

Regarding DX in general Valibot is the best thing I've used yet. I love the composability, super easy to extend with your own schemas (which is what happens largely in big projects anyway). I actually prefer the syntax over zod 👍

[fabian-hiller]

Thank you for your kind words. We have put a lot of work into the mental model, philosophy, consistency and documentation of your API. It is great to hear that this is recognised. 🙏

If I am not mistaken, schema functions (including nonOptional) only checked the input value, never the output. So I'm not sure if we should make an excuse for nonOptional. To prevent the output from being undefined or any other value, I would recommend using our pipeline feature. Here is an example. Try it out in this playground.

import * as v from 'valibot';

const OptionalOutput = v.pipe(
  v.string(),
  v.transform((input) => input || undefined),
);

const NonOptionalOutput = v.pipe(
  OptionalOutput,
  v.check((input) => input !== undefined),
);

[fabian-hiller]

For others... the bug related to this issue is the wrong output type returned by nonOptional. This is a known problem and already marked in our source code. The actual runtime implementation is currently intended and not a bug.

[fabian-hiller]

I have been investigating the output type of nonOptional and other schemes. The problem is that TypeScript can't know what the runtime code of your transform action is doing. It can't know that you're deliberately transforming a particular value to undefined, and we can't recalculate the output type of the whole pipeline if the input type changes. So I guess this is an unsolvable problem with our current implementation.

Basically, we now have 4 options if we can't solve it otherwise:

1. Remove nonOptional and other schemas because of this (probably a bad idea)
2. Restrict how nonOptional and other schemas can be used (e.g. disallow pipe)
3. Do not remove undefined when we detect a pipeline or transformation action
4. Ignore this type bug, but document clearly that this is a known issue

I need to think more about this. But at the moment I still think that option 4 is probably the best, because it is very rare that people combine nonOptional (or optional with a default value) with a transformation action that transforms certain values to undefined. Option 3 would often return undefined when the user expects and also knows that the output is never undefined. Options 1 and 2 limit the functionality of the library for common cases, just to be "safe" from the rare wrong case. Please let me know your opinion.

[NiklasPor]

So in general this problem is the same for nonOptional, nonNullable, nonNullish – either for undefined or null.

1. & 3. I don't think any of these two options would be acceptable DX wise.
2. If we disallow pipe here, would it still be allowed in optional? Otherwise we'd have the same problem with nonOptional(optional(pipe(..)).
3. I think this option seem suitable, but on enterprise level I wouldn't find this acceptable. Let's imagine one team "maintaining" the core library of an application and another team consuming it. Now the core team uses a transform to undefined, (without any nonOptional or anything). The app team, which consumes the code from the core team, will just import the base schema from the core lib and then modify it with any actions with are valid. "Oh this might be undefined, so let's put a nonNullish on it because that's not allowed here" – I don't find it realistic that everyone will check the source of the schema, if the types suggest something else.

As you said this seems like an edge-case and the typing already suggests that the output is already checked. So would it be such a problem to update the respective "non-" functions to also check outputs? I can't really see why anyone would have problems with the change, because the current typings wouldn't allow anyone to consume the undefined value anyway – and also only the behavior in this edge-case will change.

For me the mental model with these functions is like this:

• Actions in pipe always take the previous result and easier narrow the requirments down further or transform it
• Wrapper functions (like optional) seem to me like they would be able to add behaviour both in front and in the back of the passed schema. So I personally don't see much of a problem with this behaviour, but you obviously have better insights.

---

For my motivation for this in general. I wanted to build a base string type, which does not allow for empty strings or just whitespace and instead returns undefined. Then once I have my custom "Label" type or whatever I call it, I'd like to compose it with the defaults from the library.

So this is my puzzle:

```ts // Infers: string | undefined const Label = v.pipe(v.string(), v.transform(val => val.trim() ? undefined : val.trim()); // Now when someone else wants to consume this type, they just now Label -> string | undefined // So the natural action would be to use v.nonOptional() to make it requried const Person = v.object({ // Name should be required, but also behave like our base label name: v.NonOptional(Label) }) ``` There's probably an easier way to do this, I just didn't think of it first yet 😄

[fabian-hiller]

Thank you for your feedback! Checking the output is a good idea and might be the best solution with the least drawbacks. I will think about it.

We have the same problem with optional, nullable and nullish when a default value is provided, because in this case we also "remove" undefined and/or null from the output type. But checking the output and returning an issue if the value is undefined and/or null feels somehow wrong for these schemas. 🙃

[NiklasPor]

For the non* methods I think having the output check at the end would be awesome 👍

---

For optional etc. I don't have such a strong opinion, as I'm not using fallbacks that much yet.

Maybe an interesting fact, but I'd have expected the optional(schema, fallback) syntax to just short-circuit the evaluation and return the fallback. From a mental perspective just like an guard clause / early return:

if (value === undefined) {
  return fallback;
}
....

I also wrote it down as a custom optional function for valibot. Check the playground here.

Show valibot custom optional example:

```ts import * as v from 'valibot'; type UnknownSchema = v.BaseSchema>; const Wrapped = v.pipe(v.string(), v.nonEmpty()); // What I initially expected optional to do: function myOptional( schema: TSchemas[0], fallback: TFallback, ) { return v.pipe( v.union([v.undefined(), schema]), v.transform((value) => (value === undefined ? fallback : undefined)), ); } // Short-circuits the evaluation and just returns "" const customOptionalResult = v.safeParse( myOptional(Wrapped, ''), undefined, ); // Fails because the inner type does not allow empty strings const optionalResult = v.safeParse(v.optional(Wrapped, ''), undefined); console.log({ customOptionalResult, optionalResult }); ```

Just as some input for the optional operators, as I said, I don't have strong opinion about this 😁

[fabian-hiller]

Thank you very much for your feedback! I will think about it and get back to you in the next few days.

[fabian-hiller]

I am still unsure what to do with optional, nullable and nullish. I would like to find a unified solution for both. It seems that Zod has a similar problem. I will contact Colin to discuss this.

Maybe an interesting fact, but I'd have expected the optional(schema, fallback) syntax to just short-circuit the evaluation and return the fallback.

The second argument for optional, nullable and nullish is a default input value rather than a fallback output value. For such cases we have fallback, but it behaves a bit differently as it returns the fallback output value whenever an issue is returned. You can read more here:

• https://valibot.dev/guides/optionals/
• https://valibot.dev/api/fallback/

[fabian-hiller]

I think I have found a solution! 🙏 Thanks again for reporting this. I will get back to you in the next few days.

[fabian-hiller]

Fixed! 😎 v1.0.0-beta.5 is available!

[NiklasPor]

Awesome, if you're ever in the Ruhrgebiet in Germany let me invite you to a coffee / dinner / beer ❤️