fabian-hiller / valibot

The modular and type safe schema library for validating structural data 🤖
https://valibot.dev
MIT License
6.33k stars 204 forks source link

Type issue with nested generic type #878

Closed hanakannzashi closed 3 weeks ago

hanakannzashi commented 1 month ago

I don't understand why TypeScript tell me a error, I think this is a common use case

type
fabian-hiller commented 1 month ago

Can you provide a playground link? I am sure this is fixable. https://valibot.dev/playground/

hanakannzashi commented 1 month ago

Can you provide a playground link? I am sure this is fixable. https://valibot.dev/playground/

I guess issue is caused by wrong usage of never in TDefault, no type can extends never except itself.

https://valibot.dev/playground/?code=JYWwDg9gTgLgBAKjgQwM5wG5wGZQiOAcg2QBtgAjCGQgbgCh6YBPMAUzgGUBjACzZDI4AXkwA6AEJo2PfoIA8AVwB2Aa2UQA7soA0cFeq27xU1GwCSqVIrZK1G7QD5HDethXcYwCMrghmsgLI8pxwbAAeMGzKACbogYKOABSofEEAXFwAlHAA3vRwcFBsMIpQvhhiYMDsSZUQYF4+ZClpgll6lWncqklJOcKOcDBQNllZDAC+jNw+qPCpckKi-gnIdWLKiiAUbFD9EzNzEKRsYqQQAOYbYMhQZq1LeioxbNjAymwx4wyzyqgnM4Xa6VW73NiPIJ6ACMP3oQA

fabian-hiller commented 1 month ago

I would like to replace never with undefined, but the problem is that this does not work for our nullish schema because there is a difference between providing undefined as a default value and providing no default value. Providing undefined as the default for nullish would change every null input to undefined. That's why we can't specify .default as undefined, because that would lead to a bug with its output type. I know that never is a hacky solution, but I don't know what else to do. Do you have any idea?

In the meantime you have two workarounds. You could add undefined as a default value or you could ignore the TS error and cast the output type.

import * as v from 'valibot';

function mySchema<T extends v.GenericSchema>(schema: T) {
  return v.pipe(v.optional(schema, undefined), v.check(() => true));
}

const schema = mySchema(v.number());
import * as v from 'valibot';

function mySchema<T extends v.GenericSchema>(schema: T) {
  // @ts-expect-error
  return v.pipe(
    // @ts-expect-error
    v.optional(schema),
    v.check(() => true),
  ) as v.SchemaWithPipe<
    [
      v.OptionalSchema<T, never>,
      v.CheckAction<v.InferOutput<T> | undefined, undefined>,
    ]
  >;
}

const schema = mySchema(v.number());
hanakannzashi commented 1 month ago

I would like to replace never with undefined, but the problem is that this does not work for our nullish schema because there is a difference between providing undefined as a default value and providing no default value. Providing undefined as the default for nullish would change every null input to undefined. That's why we can't specify .default as undefined, because that would lead to a bug with its output type. I know that never is a hacky solution, but I don't know what else to do. Do you have any idea?

In the meantime you have two workarounds. You could add undefined as a default value or you could ignore the TS error and cast the output type.

import * as v from 'valibot';

function mySchema<T extends v.GenericSchema>(schema: T) {
  return v.pipe(v.optional(schema, undefined), v.check(() => true));
}

const schema = mySchema(v.number());
import * as v from 'valibot';

function mySchema<T extends v.GenericSchema>(schema: T) {
  // @ts-expect-error
  return v.pipe(
    // @ts-expect-error
    v.optional(schema),
    v.check(() => true),
  ) as v.SchemaWithPipe<
    [
      v.OptionalSchema<T, never>,
      v.CheckAction<v.InferOutput<T> | undefined, undefined>,
    ]
  >;
}

const schema = mySchema(v.number());

I think this is because the definitions of null and undefined as a value or as a symbol of non-existence are unclear. I think null should always be treated as a value and undefined should be treated as a non-existence symbol (except within UndefinedSchema). Thus, default should ONLY work for undefined, not for null. Actually, Zod handles it this way as well.

const nullishSchema = z.number().nullish().default(1);
console.log(nullishSchema.parse(undefined)); // 1
console.log(nullishSchema.parse(null)); // null

const nullableSchema = z.number().nullable().default(1);
console.log(nullableSchema.parse(null)); // null

Both in nullish and nullable, default not work for null, I think this is a expected behavior, not a issue.

Another example, many ORM like sequelize and TypeORM treat null and undefined separately, null means this column is a null value, undefined means this column is not selected.

Requirement and solution:

  1. If column is a null value, give a default value, ORM itself can do this.
  2. If column is not selected, give a default vaue, Parser (like zod and valibot) can do this, but currently I can only use zod z.number().nullish().default(0) because valibot v.nullish(v.number(), 0) will break this behavior.

In summary (in valibot):

  1. nullable doesn't need default param
  2. nullish default should ONLY work for undefined, not for null

If you still think a default value is necessary for null, you should treat Default Value For Optional Value and Default Value For Nullable Value separately, seems tricky

fabian-hiller commented 1 month ago

Thank you for your recommendation! I like your idea and will think about it!

fabian-hiller commented 3 weeks ago

I am currently investigating your suggestion and think I will follow it.

fabian-hiller commented 3 weeks ago

v1.0.0-beta.3 is available