Skip to content

anyOf should not result in intersection type #894

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

Closed
MunifTanjim opened this issue Apr 7, 2022 · 6 comments · Fixed by #895
Closed

anyOf should not result in intersection type #894

MunifTanjim opened this issue Apr 7, 2022 · 6 comments · Fixed by #895
Labels
question Further information is requested

Comments

@MunifTanjim
Copy link
Contributor

MunifTanjim commented Apr 7, 2022

anyOf is equivalent to OR, i.e. TypeScript's union | operator.

The current behavior is incorrect.


Related Note:

oneOf is equivalent to XOR, no direct equivalent for TypeScript. Currently treated as OR.

@drwpow
Copy link
Contributor

drwpow commented Apr 24, 2022

Hm, from the OpenAPI specification:

OpenAPI 3.0 provides several keywords which you can use to combine schemas. You can use these keywords to create a complex schema, or validate a value against multiple criteria.

  • oneOf – validates the value against exactly one of the subschemas
  • allOf – validates the value against all the subschemas
  • anyOf – validates the value against any (one or more) of the subschemas

I don’t believe anyOf is OR, as it may be either one of the subschemas, or all of the subschemas. To get OR you would want oneOf, no?

I think the current behavior is correct (Partial<A> & Partial<B> & Partial<C>) unless I’m missing something.

@drwpow drwpow added the question Further information is requested label Apr 24, 2022
@MunifTanjim
Copy link
Contributor Author

MunifTanjim commented Apr 24, 2022

Say you want to define a parameter that accepts a number or string. If you do number & string or Partial<number> & Partial<string> in TypeScript, both of them result in never. Which is clearly not what we want.

Now think of this type in TypeScript: type Str = string | 'one'. Any string value (including 'one') will satisfy the Str type. It's not exclusively oneOf string or 'one', because in that case 'one' is an impossible value (it can't be exclusively 'one' without being a string).

From the OpenAPI's specification, oneOf is Exclusive OR - which can't be described easily in TypeScript without writing some complex custom types. And anyOf is OR - which is essentially equivalent to the union operator |.

Does the explanation sound reasonable?

@MunifTanjim
Copy link
Contributor Author

Here's another example.

With the current behavior:

paths:
  /test:
    post:
      requestBody:
        content:
          application/json:
            schema:
              anyOf:
                - $ref: '#/components/schemas/A'
                - $ref: '#/components/schemas/B'
      responses:
        '200':
          description: ''
components:
  schemas:
    A:
      type: object
      properties: 
        type: 
          type: string
          enum: ['a']
        a: 
          type: number
      required:
        - type
        - a
          
    B:
      type: object
      properties:
        type: 
          type: string
          enum: ['b']
        b: 
          type: string
      required: 
        - type
        - b

would result in:

type A = {
  type: 'a',
  a: number
}

type B = {
  type: 'b',
  b: string
}

type X = Partial<A> & Partial<B>

// testing
const x1: X = {}
const x2: X = { a: 1 }

If a parameter accepts anyOf type A or type B, the TypeScript types generated by openapi-typescript is Partial<A> & Partial<B>, which passes for:

  • {} - but empty object is not accepted at all.
  • { a: 1 } but a: number is only supported when type: 'a'.

I think the current behavior is correct (Partial & Partial & Partial)

So the current behavior is giving false positives.

@drwpow
Copy link
Contributor

drwpow commented May 2, 2022

Yes I see the improvement that could be made here. That short summary I posted earlier is a bit confusing, but reading more into it, it does seem that anyOf requires it to validate against at least one of the schemas which Partial<> would indeed prevent, as you pointed out. I now agree with you that Partial<> should go no matter what.

So in digging into this a little more, I learned a little more about TypeScript union types in relation to objects, and they behave a little differently than expected. For example:

type AnyOf = string | { a: string } | { b: string };

const testA: AnyOf = 'foo';                 // ✅ valid
const testB: AnyOf = { a: 'foo' }           // ✅ valid
const testC: AnyOf = { b: 'foo' }           // ✅ valid
const testD: AnyOf = { a: 'foo', b: 'foo' } // ✅ valid

It seems that TypeScript unions provide proper anyOf behavior on their own, without modification. So I think just a simple union type would suffice for anyOf?

However, this does pose a problem for oneOf, because that change would mean their generated types would be exactly the same when we know they shouldn’t be:

type OneOf = string | { a: string } | { b: string };

const testA: OneOf = 'foo';                 // ✅ valid
const testB: OneOf = { a: 'foo' }           // ✅ valid
const testC: OneOf = { b: 'foo' }           // ✅ valid
const testD: OneOf = { a: 'foo', b: 'foo' } // ❌ this is valid but shouldn’t be

👆 This is current behavior, by the way, which is incorrect! I’m not sure how to solve this problem; one solution I found was to provide never for all keys, like so:

type OneOf = string | { a: string; b: never } | { a: never; b: string };

const testD: OneOf = { a: 'foo', b: 'foo' } // ✅ this now fails the typecheck

It seems to me like anyOf should change to just a simple union type, like oneOf currently implements. However, oneOf’s behavior should also change to not allow intersections to happen like they do currently. Whether that’s the never approach or a better one, I’m not sure. And I’m tempted to make the latter a separate scope of work entirely (especially if it’s breaking behavior).

Does this proposal sound reasonable?

@MunifTanjim
Copy link
Contributor Author

Does this proposal sound reasonable?

Yep, absolutely. Agree on both matter.

And here's the PR for fixing anyOf behavior: #895 😃

@eriktim
Copy link

eriktim commented May 25, 2022

Regarding the oneOf or xor behaviour I stumbled upon this one: https://stackoverflow.com/questions/42123407/does-typescript-support-mutually-exclusive-types/53229567#53229567.

Using those types

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

allows you to do this

type Foo = { foo: string }
type Bar = { bar: number }

type FooOrBar = Foo | Bar | string
type FooXorBar = Xor<Xor<Foo, Bar>, string> // = string | (Without<Foo, Bar> & Bar) | (Without<Bar, Foo> & Foo)

const or1: FooOrBar = { foo: 'sdf' } // ok
const or2: FooOrBar = { bar: 123 } // ok
const or3: FooOrBar = 'ddd' // ok
const or4: FooOrBar = { foo: 'sdf', bar: 123 } // ok
const or5: FooOrBar = {} // errors correctly

const xor1: FooXorBar = { foo: 'sdf' } // ok
const xor2: FooXorBar = { bar: 123 } // ok
const xor3: FooXorBar = 'ddd' // ok
const xor4: FooXorBar = { foo: 'sdf', bar: 123 } // errors correctly
const xor5: FooXorBar = {} // errors correctly

Not sure how feasible it is as the Xor only works on two types and the order matters, e.g. type FooXorBar = Xor<Foo, Xor<string, Bar>> will only result in Foo | Bar | string as string is not an object. I do not know enough about the implementation of this package to estimate whether that could work or not. But hopefully this adds on getting to a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants