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

Getting concrete errors in unions is too hard #216

Closed
danielo515 opened this issue Oct 19, 2023 · 18 comments
Closed

Getting concrete errors in unions is too hard #216

danielo515 opened this issue Oct 19, 2023 · 18 comments
Assignees
Labels
enhancement New feature or request priority This has priority

Comments

@danielo515
Copy link

Hello, first of all, thank you for the work you are putting into this library.
Currently I am trying to use it to validate an array of objects that can be of a variety of shapes, hence I use union to specify all the possible types.

When I get an error in a "first level" I get a quite readable message using my very basic "flatten function". Something like: field name should not be empty at 4.name

However, when the error happens at a nested property, despite I am specifying proper specific errors for everything I get a very generic message: Invalid type at 4.input

I think the cause is the usage of union. Here is a basic example:

function nonEmptyString(name: string) {
    return string(`${name} should be a string`, [toTrimmed(), minLength(1, `${name} should not be empty`)])
}

const BasicInputSchema = object({ type: FieldTypeSchema });
const MultiSelectNotesSchema = object({
    type: literal("multiselect"), source: literal("notes"),
    folder: nonEmptyString('multi select source folder') // can't reach this message
});
const MultiSelectFixedSchema = object({ type: literal("multiselect"), source: literal("fixed"), multi_select_options: array(string()) });
export const MultiselectSchema = union([MultiSelectNotesSchema, MultiSelectFixedSchema]);

const InputSelectFixedSchema = object({
    type: literal("select"),
    source: literal("fixed"),
    options: array(object({
        value: nonEmptyString('Value of a select option'), // can't reach this message
        label: string()
    }))
});

const InputTypeSchema = union([
    BasicInputSchema,
    MultiselectSchema,
    InputSelectFixedSchema
]);

const FieldDefinitionSchema = object({
    name: nonEmptyString('field name'), // This is fine
    label: optional(string()),
    description: string(),
    input: InputTypeSchema
});

const FieldListSchema = array(FieldDefinitionSchema);

export function validateFields(fields: unknown) {
    const result = safeParse(FieldListSchema, fields);

    if (result.success) {
        return []
    }
    console.error('Fields issues', result.issues)
    return result.issues.map(issue =>
        `${issue.message} at ${issue.path?.map(item => item.key).join('.')}`
    );
}

I will be very grateful if you either provide a nicer way to reach this errors or a workaround to implement it myself.

Cheers

@danielo515
Copy link
Author

The reason I suspect the problem is using the union type is because

  1. the lack of proper information at the root
  2. the ridiculous and useless long issue.issues array that I don't understand:
[
    {
        "reason": "type",
        "validation": "union",
        "origin": "value",
        "message": "Invalid type",
        "input": "dataview",
        "issues": [
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview"
            }
        ],
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "type",
                "value": "dataview"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "input": "dataview",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "type",
                "value": "dataview"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "string",
        "origin": "value",
        "message": "folder name should be a string",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "folder"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "input": "dataview",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "type",
                "value": "dataview"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "number",
        "origin": "value",
        "message": "Invalid type",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "min"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "number",
        "origin": "value",
        "message": "Invalid type",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "max"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "input": "dataview",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "type",
                "value": "dataview"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "source"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "string",
        "origin": "value",
        "message": "folder name should be a string",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "folder"
            }
        ]
    },
    {
        "reason": "string",
        "validation": "min_length",
        "origin": "value",
        "message": "dataview query should not be empty",
        "input": "",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "query",
                "value": ""
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "input": "dataview",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "type",
                "value": "dataview"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "literal",
        "origin": "value",
        "message": "Invalid type",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "source"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "array",
        "origin": "value",
        "message": "Invalid type",
        "path": [
            {
                "schema": "object",
                "input": {
                    "type": "dataview",
                    "query": "dv.pages('#person').map(p=>p.file.name)"
                },
                "key": "options"
            }
        ]
    },
    {
        "reason": "type",
        "validation": "union",
        "origin": "value",
        "message": "Invalid type",
        "input": {
            "type": "dataview",
            "query": "dv.pages('#person').map(p=>p.file.name)"
        },
        "issues": [
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "type",
                        "value": "dataview"
                    }
                ]
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "source"
                    }
                ]
            },
            {
                "reason": "type",
                "validation": "string",
                "origin": "value",
                "message": "multi select source folder should be a string",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "folder"
                    }
                ]
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "input": "dataview",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "type",
                        "value": "dataview"
                    }
                ]
            },
            {
                "reason": "type",
                "validation": "literal",
                "origin": "value",
                "message": "Invalid type",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "source"
                    }
                ]
            },
            {
                "reason": "type",
                "validation": "array",
                "origin": "value",
                "message": "Invalid type",
                "path": [
                    {
                        "schema": "object",
                        "input": {
                            "type": "dataview",
                            "query": "dv.pages('#person').map(p=>p.file.name)"
                        },
                        "key": "multi_select_options"
                    }
                ]
            }
        ]
    }
]

@fabian-hiller
Copy link
Owner

The problem with union is that here we need to structure the issues here as subissues, since they can contradict each other and the error messages can be confusing as a result. flatten currently ignores subissues.

For example, if your union scheme looks like to the following code, it would be confusing to show the error messages of string and number to the user. Therefore, we display only the error message of union.

const UnionSchema = union(
  [string('Please enter a string.'), number('Please enter a number.')],
  'Please enter a string or number.'
);

However, I am aware that this is not optimal for objects. So I am interested in improving this. Do you have any ideas? Do you know how this works with other schema libraries? Feel free to create minimal code examples with your desired API.

@fabian-hiller fabian-hiller self-assigned this Oct 19, 2023
@fabian-hiller fabian-hiller added the question Further information is requested label Oct 19, 2023
@danielo515
Copy link
Author

Yes, I see your point, and makes complete sense, I was thinking only in my particular use-case.
What I think is needed in this case, it is a different combinator. Union, as you pointed out, can end having contradictory types, a tagged-union however should uniquely point you to one specific type through it's tag.
So my proposal is you add a new method, called however you want (io-ts calls it sum) where you define your tag key then you can just show the error of the type whose tag matches or just the sum error in case none matches:

sum('type', // This is the tag, all members must have it
    [object({type: 'A'}), object({type: 'B', something: string()}], // Sum of schemas
    'X does not match any valid input' // Fallback message
)

@fabian-hiller
Copy link
Owner

Thank you for your answer! Yes, we need a solution like this. Seems similar to Zod's z.discriminatedUnion or?

@danielo515
Copy link
Author

Yes, here is another example: https://github.com/gcanti/io-ts/blob/master/Decoder.md#the-sum-combinator

For me the critical part is extracting the proper errors, just in case I was not clear about that

@iamriajul
Copy link

iamriajul commented Oct 21, 2023

Is this related related:
Cannot read properties of undefined (reading 'map')

image
image

This error is thrown only when client-side validation fails, if client-side validation passes then everything works properly.
*** Note: If I disable Javascript then validation works properly without any issues ***

Context: I'm using it with Qwik + ModularForms + ValiBot + Union

My Union implementation:

const RegisterSchema = union([
  object({
    type: literal("send-otp"),
    phone: string([
      minLength(1, 'Please enter your phone number.'),
      regex(/^\+?(?:[0-9] ?){6,14}[0-9]$/, 'Please enter a valid phone number.'),
    ]),
    id: optional(string([
      regex(/^[0-9]+$/, 'Illegal state, please try again.'),
    ])),
  }),
  object({
    type: literal("register"),
    id: string([
      regex(/^[0-9]+$/, 'Illegal state, please try again.'),
    ]),
    code: string([
      custom(
        value => !!value.toString().match(/^[0-9]{6}$/),
        'Please enter a valid OTP.'
      ),
    ]),
    phone: string([
      minLength(1, 'Please enter your phone number.'),
      regex(/^\+?(?:[0-9] ?){6,14}[0-9]$/, 'Please enter a valid phone number.'),
    ]),
    email: optional(string([
      regex(/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/, 'Please enter a valid email address.'),
    ])),
    name: string([
      minLength(1, 'Please enter your name.'),
      minLength(3, 'Please enter a valid name.'),
    ]),
  }),
]);

@danielo515
Copy link
Author

danielo515 commented Oct 21, 2023 via email

@fabian-hiller
Copy link
Owner

It is partially related. With Modular Forms, I assumed that an object is always passed as a the outermost schema. So there is always a path. If you now use union as the outermost schema instead of object, this is no longer the case. Again, we need a solution like discriminatedUnion or switch that allows better error management. Then this problem will not occur anymore.

@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority and removed question Further information is requested labels Oct 21, 2023
@iamriajul
Copy link

I recommend you turn strict mode in your typescript

I already have a strict mode on in my tsconfig

@iamriajul
Copy link

Again, we need a solution like discriminatedUnion or switch that allows better error management.

Is there any plan to develop discriminatedUnion any time soon?

@fabian-hiller
Copy link
Owner

Yes, I'm in the process of weighing the options right now.

I find a switch API as described here to be very useful, but also problematic. The reason for this is that the schemas inside the function of the first argument are not created until validation, and thus are not accessible in advance. This causes problems with PR #211, for example. An alternative to this would be to add the ability to dynamically select the schema to be used to the current union implementation.

const Schema = union(
  [
    object({ type: literal('a'), foo: number() }),
    object({ type: literal('b'), bar: boolean() }),
  ],
  (input, [a, b]) => (input?.type === 'a' ? a : b)
);

The biggest problem here is that input is of type unknown and TypeScript will throw an error on input?.type. The solution would be to pass a general schema like object({ type: string() }, unknown()) as another argument that applies to all options of the union to make the input of the function for type safe. However, this makes the API quite complicated and less powerful.

My current assumption is to add discriminatedUnion (at least experimentally). I will have a look at the implementation soon.

@fabian-hiller
Copy link
Owner

I have implemented discriminatedUnion. The API is expected to be available in the next release in a few days.

@danielo515
Copy link
Author

danielo515 commented Oct 24, 2023 via email

@danielo515
Copy link
Author

@fabian-hiller I don't see any reference to the discriminated union in the latest release. Was it included?

@fabian-hiller
Copy link
Owner

It is not public yet. I will try to publish it over the weekend.

@danielo515
Copy link
Author

For those interested, the way I'm working around this is by creating intermediate more permissive schemas. For example, the fields that are unions I have them as unknown, then I specify them in a derived schema. Then I perform a quick validation with the final schema, and if that fails I start from the more permissive to the more restrictive wrapping in my own errors

@fabian-hiller
Copy link
Owner

@danielo515 thank you for the info. Can you provide a code example?

I plan to release discriminatedUnion today or tomorrow. Until then, you can also copy the source code into your project to be able to use it.

@fabian-hiller
Copy link
Owner

v0.20.0 is now available

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

No branches or pull requests

3 participants