gristlabs / ts-interface-checker

Runtime library to validate data against TypeScript interfaces.
Apache License 2.0
323 stars 18 forks source link

`strictCheck` fails when using a parent interface to check a child object #46

Closed alexmojaki closed 3 years ago

alexmojaki commented 3 years ago

Extending the test it should handle enum-based discriminated unions a bit:

    const circle = {kind: enumUnion.ShapeKind.Circle, radius: 0.5};
    Circle.check(circle);
    Circle.strictCheck(circle);
    Shape.check(circle);
    Shape.strictCheck(circle);

The first three checks are fine, but the last check fails with value.radius is extraneous. It's clear why this happens, I'm just not sure if this is actually a problem or if the advice here should just be "don't use strictCheck".

Note that this is not a problem for it should handle discriminated unions which uses shapes-ti.ts where Shape is a union rather than a parent interface. It seems to me like they should both succeed, as I'd generally prefer to use interface inheritance over unions.

dsagal commented 3 years ago

Strict checks aim to copy TypeScript's own checking behavior for object literals, and I think in this case at least, they match it reasonably well. See e.g. this question: https://stackoverflow.com/questions/31816061/why-am-i-getting-an-error-object-literal-may-only-specify-known-properties. Similar workarounds as in typescript apply here as well. In this case, if extraneous properties don't matter, using check() is great. If they do matter, then a discriminated union would address it (it can be used in addition to inheritance, don't need to be either-or).

Plus, I don't see a general-purpose solution here if the checker only knows about the parent type. StrictCheck means it shouldn't allow a Shape like, say, {kind: enumUnion.ShapeKind.Circle, legs: 6, reflectivity: 'shiny'}. But how can it know whether there happens to be just such a type that derives from Shape?

alexmojaki commented 3 years ago

Strict checks aim to copy TypeScript's own checking behavior for object literals, and I think in this case at least, they match it reasonably well. See e.g. this question: https://stackoverflow.com/questions/31816061/why-am-i-getting-an-error-object-literal-may-only-specify-known-properties.

That's fascinating, I didn't know TypeScript did that. I was very surprised at first but the reasoning make sense. I can see how users might want similar protection from typos and other mistakes from this library and how strict checks provide that, so emulating it makes sense.

I'm less convinced that providing a Circle where only a Shape is needed is a mistake that needs to be protected against, but that's highly debatable and this is all hypothetical until an actual user complains. Since this isn't an actual bug, I'm closing this.

If they do matter, then a discriminated union would address it (it can be used in addition to inheritance, don't need to be either-or).

Interesting idea. If I understand correctly, you'd have two similar but separate types: the parent interface, and a union of all the children. It's not bad, but maintaining a union like that can be annoying.

Plus, I don't see a general-purpose solution here if the checker only knows about the parent type. StrictCheck means it shouldn't allow a Shape like, say, {kind: enumUnion.ShapeKind.Circle, legs: 6, reflectivity: 'shiny'}. But how can it know whether there happens to be just such a type that derives from Shape?

It can't, but intuitively I expect it to recognise subtypes in the suite it's been given. But that's not necessarily a good reason.