typescript union should be implemented as anyOf

[fantapop]

Digging through the openapi docs, I ran across this:

https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/#anyof-vs-oneof

It seems that oneOf is an exclusive OR. I believe the correct thing to use for unions is anyOf.

Sorting

• I'm submitting a ...

  • bug report
  • feature request
  • support request

• I confirm that I

  • used the search to make sure that a similar issue hasn't already been submit

Context (Environment)

Version of the library: 3.0.5
Version of NodeJS: 12.12

• Confirm you were using yarn not npm: [x]

[WoH]

There is no perfect equivalent, but here is why oneOf was chosen:

interface LeftType {
    value1: number,
}
interface RightType {
    value2: number
}

type ExampleUnion = LeftType | RightType;

If you try to assign {value1: "", value2: ""}, TS will complain and so will tsoa if you throw on additional Properties.

Edit: I have changed the Union definition to actually show the issue.

[fantapop]

I wasn't aware of this until doing some tests just now, but this is actually valid. There's no errors here.

type U = { value1: string } | {value2: string};
const v: U = {value1: "it", value2: "works"};
console.log(v);

There's some info about it here:
https://devblogs.microsoft.com/typescript/announcing-typescript-3-5-rc/#improved-excess-property-checks-in-union-types

Is TSOA validating this as a disjointed union?

Before learning that typescript is loose with its unions, my concern with oneOf was about overlapping types. These are somewhat contrived examples but I think easier to reason about than an example with objects.

type Five = 5;
type FiveOrNumber = Five | number;

this would get sent over the wire something like:

FiveOrNumber:
  oneOf:
    - $ref: '#/components/schemas/Five'
    - type: number

According to the open api docs this type should match any number except for 5 because in the case of 5 it will match both the type number and the type Five. In the typescript version that was written in tsoa it will match any number including the number 5.

Here's a similar one with objects:

interface Cat {
  name: string;
  type?: CatBreed;
}
interface Dog {
  name: string;
  type?: DogBreed;
}
type Pet = Cat | Dog;

If someone passed in the following valid request:

{
  "name": "bubbles"
}

using a oneOf should deny the request because it matches both Cat and Dog
use an anyOf should allow the request because it matched one or more.

I don't know if this is how you think about TSOA, but to me, an ideal case is that typescript is created in TSOA, then converted to openapi, then if a typescript client is generated from the spec, its as close as possible to the original. I realize that getting this perfect will never occur partially because of the expressiveness of the language but also due to ambiguities in how things are implemented.

[WoH]

Please read the discussion we had here: #400 (comment)

You're right about TS not complaining because their excess property checks are sometimes not as strict as you'd expect and want yet (for example in the sample I initially used, which was not the one I should've gone for and subsequently changed it to better reflect the issue).
The way I interpreted the Article is that it's not valid, but unchecked.

If you have additional concerns, feel free to bring them up.

I'm not sure about TS, but it seems like the Typechecker merges the declarations (for performance reasons) and then struggles to check excess properties. We "save" the member definitions so we can match against each one (we only have to check once at the system boundary, so a first-match approach is fine here).
It's important here to remember that tsoa allows an additionalProp config. If you "silently ignore" additional Properties you'd get pretty close to the old way TS was validating unions.
If you "throw on excess", we match only against the first exact match without excess/additional properties.

[fantapop]

Is this the comment you’re referring to? #400 (comment) There is a typo in the example @dgreene1 included as justification. Notice that the validation is failing because`value` is not a member of either of the types. It should be `value2` in that example. I just want to put out there that this is not a current pain point for me. I just noticed because I was trying to get nullable behavior working better for me. I think the end goal should be about how faithfully code generated from the resulting open api spec follows the intentions of the type that generated it. Typescript-fetch generator does not support oneOf or anyOf yet. It will be a bigger problem if code generated from a spec using `oneOf` is translated to some kind of disjoint union which more closely matches the tune of the OA3 spec examples. If oneOf is implemented as a typescript union then they’re both the same and not a problem for typescript users. I wonder what code generation in other languages that support this concept do. I’ll see if I can track something down.

…

On Sat, Apr 25, 2020 at 5:36 AM Wolfgang Hobmaier ***@***.***> wrote: Please read the discussion we already had. #400 <#400> If you have additional concerns, feel free to bring them up. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#671 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABSZXKFZUUANCWGCLKEYEATROKVIZANCNFSM4MPZINEQ> .

[WoH]

There is a typo in the example @dgreene1 included as justification. Notice
that the validation is failing becausevalue is not a member of either of the types. It should be value2 in that example.

I didn't even notice that, oops.
Certainly would've tried pushing for anyOf in that case. I'd be curious to see what the other codegens are doing. Classifying this as a bug for now.

[WoH]

I'll have to think about the validation implications of this, but maybe we can live without changes for now. If you allow excess props you're already ok, if you wanna throw on excess props I'd assume you'd intuitively want to have the excess prop validation to be as strict as possible and not allow mixed schemas. However, that's the point where we would diverge from OpenAPI and what TS currently does which should not be the case.

[fantapop]

I found this open issue. If you read through, there’s clearly a lot of issues implementing this in many languages. Notably this comment sums it up

OpenAPITools/openapi-generator#15 (comment)

I’m curious to see where all this ends up. I personally feel like the spec is pretty explicit about oneOf only allowing one and only one match. This is a useful construct For matching parameter objects of different types. I’m surprised typescript doesn’t have better first class support for this.

[fantapop]

here is a post I found informative on the subject. They even call the final product they end up with OneOf

https://timhwang21.gitbook.io/index/programming/typescript/xor-type

[derolf]

AllOf: That’s just wrong.

[WoH]

That’s just wrong.

That's what I thought. But I'm sure once you read this discussion and the referenced threads, you'll understand how TypeScript handles union types by default and the implied complexity of oneOf validation without discriminators you seem to be suggesting. But be careful, you may eventually end up changing your preference (like me) and start leaning towards modeling TS Unions as anyOf, or at least be able to provide a nuanced opinion (one which would model as oneOf only in certain circumstances). Good luck until then 🍀

[derolf]

@WoH , what I actually did was dropping TSOA and using https://github.com/Eywek/typoa . Works like a charm!

[derolf]

Btw. how do you model type AorB = { a: number, b: string } | { a: string } in TSOA?

[derolf]

Btw. reading through the thread: The alternative (better) solution to turning a type union into a weird allOf would have been to make TSOA bark if it finds a "type union" that would create a problem with oneOf.

Just imagine how a codegen would turn that allOf back into code: A | B -> TSOA -> allOf -> codegen -> Partial<A> & Partial<B>, which is completely wrong.

So, why going mad because of an artificial corner constructed above (see #671 (comment))? Why not make a good solution for 99% of the benign cases.

[fantapop]

@derolf Are you talking about allOf or anyOf? I thought allOf was the least contentious of them.

from open api spec here: allOf – validates the value against all the subschemas

In TSOA intersection is converted to allOf not union. See here

[derolf]

What about a compromise?

Usually, when one uses a union, the rational is most of the time indeed to model some xor-kind of relationship.

If TSOA detects “conflicting” properties that rule out that two subtypes can coexist, it could generate oneOfs.

[fantapop]

@derolf Can you give an example of the type and what you're hoping the outcome of the TSOA transformation would be? The code that transforms unions has a number of different cases depending on how many there are and whether they're nullable or not. It's somewhat complex. Another ideal is that the generated output can then be run through the OpenAPI codegen process to regenerate the same or functionaly equivalent code. This is not always possible currently but hopefully it should get better as Open API and typescript become more expressive.