Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type issue with nested generic type #878

Closed
hanakannzashi opened this issue Oct 17, 2024 · 7 comments
Closed

Type issue with nested generic type #878

hanakannzashi opened this issue Oct 17, 2024 · 7 comments
Assignees
Labels
fix A smaller enhancement or bug fix priority This has priority

Comments

@hanakannzashi
Copy link

hanakannzashi commented Oct 17, 2024

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

type
@fabian-hiller
Copy link
Owner

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

@fabian-hiller fabian-hiller self-assigned this Oct 17, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label Oct 17, 2024
@fabian-hiller
Copy link
Owner

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
Copy link
Author

hanakannzashi commented Oct 19, 2024

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
Copy link
Owner

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

@fabian-hiller fabian-hiller added priority This has priority fix A smaller enhancement or bug fix and removed question Further information is requested labels Oct 19, 2024
@fabian-hiller
Copy link
Owner

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

@fabian-hiller
Copy link
Owner

v1.0.0-beta.3 is available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A smaller enhancement or bug fix priority This has priority
Projects
None yet
Development

No branches or pull requests

2 participants