More detailed error message for "Xyz | null" cases (not just "there are no valid alternatives")

[dko-slapdash]

Adding | null alternative significantly degrades verboseness of the error message.

Example:

createAssertType<{
  data: {
    node: { a: string; closed: string; b: string } | null;
  };
}>()({ data: { node: { a: "a", closed: false, b: "b" } } });

// Result:
// TypeGuardError: validation failed at $.data.node: there are no valid alternatives

Notice that it doesn't mention the name of the mis-typed field ("closed"), so it's pretty hard to debug, what's the problem (especially when there is a lot of fields).

If I remove | null clause, it starts showing the field name:

createAssertType<{
  data: {
    node: { a: string; closed: string; b: string };
  };
}>()({ data: { node: { a: "a", closed: false, b: "b" } } });

// Result:
// TypeGuardError: validation failed at $.data.node.closed: expected a string

Is it (at least theoretically) possible to improve error messages here? The case when | null or | undefined or field?: type are used is pretty common (e.g. in GraphQL responses, the field is often times nullable).

[dko-slapdash]

@woutervh- If you're positive on this | null idea, I can try implementing it by myself and submit a PR. Looks like I anyway need to fork the library for our purposes, so would be happy to contribute!

[woutervh-]

Hi @dko-slapdash

Yes, in theory it's possible to improve the error messages. But to me it's not entirely clear what should be reported in the error message. One idea I had is to report the problem with each alternative in the type unions, but this can rapidly grow the error message in size and complexity. Especially when there are nested alternatives, does each possible branch need to be reported?

Any input on some sensible error reporting would be appreciated.

[dko-slapdash]

report the problem with each alternative in the type unions

Since | null and | undefined (which is almost the same as ?field) alternatives are used very frequently, I think it would be enough to process just them separately.

I.e. in this particular snippet:

const data = { data: { node: { a: "a", closed: false, b: "b" } } }
createAssertType<{
  data: {
    node: { a: string; closed: string; b: string } | null;
  };
}>()(data);

the updated logic in the lambda might be:

1. OK, we have | null as one of alternatives. Is data really null? No. So remove null from the alternatives list.
2. (Do exactly the same for undefined BTW if there was | undefined alternative.)
3. Proceed with the the code which exists there now.

[woutervh-]

hi @dko-slapdash

It's an interesting idea, doing it just for null and undefined :) I will take a look if it can be done easily.

[oleg-slapdash]

@woutervh- Please review this PR https://github.com/woutervh-/typescript-is/pull/96 that partially fixes this issue. Thank you!

[woutervh-]

Released v0.18.3

[dko-slapdash]

@woutervh- Oh wow, thanks a lot! BTW, the credit should've been given to @oleg-slapdash oleg-slapdash who prepared that PR, not to me.