OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.33k stars 6.46k forks source link

[BUG][Typescript] modeling anyOf vs oneOf #6376

Open fantapop opened 4 years ago

fantapop commented 4 years ago

Bug Report Checklist

Description

I noticed there is a lack of support for the open api 3 anyOf operation in most of the typescript templates. There are quite a few implementations of oneOf using typescript unions. Based on my reading of the spec and understanding of typescript I would expected that anyOf would be implemented via a type union and oneOf would be something more complicated which doesn't have native syntax support for in typescript. I wanted to gather the thoughts of those who have worked on these implementations.

This paragraph indicates that oneOf should not be allowed to match more than one of the types listed.

I was expecting it to be implemented something more like an exclusive-or. Some google around turned up this informative blog post on the subject.

Is this differentiation useful to people? Would a change like this horribly break users of generated typescript libraries? Am I missing something here? Thanks for your time.

@petejohansonxo @eriktim @macjohnny @karismann @SAnDAnGE

To divulge a little about my interest in this, I've been using a typescript framework call TSOA which generates an open api spec using the typescript interfaces and annotations attached to the controllers of our api server. It's really a great project. We're then generating a typescript client for our admin tool using the typescript-fetch openapi generator. It's been an interesting experiment to see what typescript goes in and what comes out on the other end. We've come to a point where we need this union support working and so I've started to investigate an implementation.

amakhrov commented 4 years ago

@fantapop Do you have any real-life example where you would use oneOf in the spec without some sort of discriminator property (doesn't have to be officially marked as discriminator in the spec)? And if there is such a discriminator property, then type union in TS would model that correctly.

Point is - perhaps it's sufficient to use type union to model both anyOf and oneOf?

cnsphst commented 4 years ago

I have a real-life example. At my compony we have a service endpoint that, depending on a query parameter, returns either an object or an array of objects. We modeled the two responses with oneOf but since the response has no common property we don't have a discriminator. (Unless I'm missing something and we actually could have one)

Btw: That definition produces invalid typescript because no models are generated for the two responses.

responses:
  '200':
    description: The service returns the midents of the found projects. If multiple is 1 it returns a list of midents. If multiple is 0 it returns one mident
    content:
      application/json:
        schema:
          oneOf:
            - description: One search result. This is returned when the parameter multiple is 0
              type: object
              properties:
                error:
                  type: string
                mident:
                  type: string
            - description: List of search results. This is returned when the parameter multiple is 1
              type: array
              items:
                type: object
                properties:
                  catalog:
                    type: string
                  mident:
                    type: string
amakhrov commented 4 years ago

From TS standpoint this is totally ok to be modeled as union type.

fantapop commented 4 years ago

@amakhrov I agree that the use cases for oneOf as described in the spec are much rarer than anyOf.

The one example I can think of is one that's mentioned in the x-or blog post I linked above which is around different sets of allowed parameters. It's actually very similar to @cnsphst's example above. A search api which takes objects in the post body to describe what the search should return. If, in the post body, an id or other parameter that is unique on that data type is passed it returns a singular object format. If no id is passed but some other combinations of searchable traits which wouldn't uniquely identify, an array of objects is returned.

If we were using unions to model the POST body parameter, the generated union might something like this:

type SingleSearch = { id: string };
type MultiSearch = { age?: number; maxCount?: number }
type SearchApi = SingleSearch | MultiSearch;

without a discriminator, the SearchApi type would let through invalid combinations of fields which may not be expected on the implementation side of the api.

const good1: SearchApi = { id: '123' };
const good2: SearchApi = { };
const good3: SearchApi = { age: 15 };
const alsoAllowed: SearchApi = { id: '123', maxCount: 10 }

My general thought on the matter is that its most important that union can be modeled in a predictable way since its much more common. I also agree that using union for both is a pretty good approximation.

amakhrov commented 4 years ago

@fantapop great example, thanks. Indeed, union allows illegal combinations here. I'd love to learn about a safer way to represent it in TS.

fantapop commented 4 years ago

Apparently the issue for directly supporting syntax for exclusive or was closed because they found a reasonableish approach to supporting it with conditional types here: https://github.com/Microsoft/TypeScript/issues/14094#issuecomment-373782604

Here is the type that is suggested:

type Without<T, U> = { [P in Exclude<keyof T, keyof U>]?: never };
type XOR<T, U> = (T | U) extends object ? (Without<T, U> & U) | (Without<U, T> & T) : T | U;

It means that items in a oneOf would all need to be chained together in an XOR, something like this:

type Element = XOR<Foo, XOR<Bar, XOR<Baz, Qux>>>

It would have been really great if they had agreed on the syntactic sugur ^ for this because it would have been an obvious and easy implementation for the TSOA framework to use that as oneOf representation and then back to ^ during a conversion by open api generator.