anyOf clauses are combined via intersection (&), should be union (|)

[dbjorge]

Summary

The transformation rules for an object schema with an anyOf clause look like this:
https://github.com/drwpow/openapi-typescript/blob/8a999ebaea31140b24632f6439dc3d4a41caa319/src/transform/schema.ts#L39-L41

This should be using a union, not an intersection.

Impact

This comes up in the GitHub OpenAPI specification (see https://github.com/octokit/rest.js/issues/2000 for one example in the check/update operation).

Explanation/repro

This StackOverflow answer is a pretty thorough writeup of why a union would be more appropriate.

As an example where the current behavior is problematic, consider this case:

# repro-schema.yaml
{
  "openapi": "3.0.3",
  "components": {
    "schemas": {
      "example": {
        "type": "object",
        "anyOf": [
          {
            "properties": {
              "anyof_prop_common": { "enum": [ "a" ] },
              "anyof_prop_a": { "type": "string" },
            },
            "required": [ "anyof_prop_common" ]
          },
          {
            "properties": {
              "anyof_prop_common": { "enum": [ "b" ] },
              "anyof_prop_b": { "type": "string" },
            },
            "required": [ "anyof_prop_common" ]
          },
        ],
      },
    },
  },
}

// test.ts
import { components } from './repro-typings';

type Example = components['schemas']['example'];
let exampleValue: Example;

exampleValue = {
    // This should be accepted, but this line actually fails to compile because the type of
    // components['schemas']['example']['anyof_prop_common'] is essentially "a" & "b", ie, "never"
    anyof_prop_common: 'a',
    anyof_prop_a: 'value'
};

> npx openapi-typescript .\repro-schema.yaml -o .\repro-typings.ts
✨ openapi-typescript 3.0.0
🤞 Loading spec from .\repro-schema.yaml…
🚀 .\repro-schema.yaml -> .\repro-typings.ts [197ms]

> npx tsc .\test.ts
test.ts:10:5 - error TS2322: Type 'string' is not assignable to type 'never'.

10     anyof_prop_common: 'a',
       ~~~~~~~~~~~~~~~~~

  repro-typings.ts:11:7
    11       anyof_prop_common: "a";
             ~~~~~~~~~~~~~~~~~
    The expected type comes from property 'anyof_prop_common' which is declared here on type 'Partial<{ anyof_prop_common: "a"; anyof_prop_a?: string; }> & Partial<{ anyof_prop_common: "b"; anyof_prop_b?: string; }>'


Found 1 error.

Related cases

Separately from this bug, the same SO post I linked above has a good discussion of how to handle oneOf, which is much harder to convert in a way that is accurate. Personally, I think it is more valuable for this library to produce typings that are over-accepting than under-accepting, so based on the tradeoffs that SO post talks about, I would recommend keeping the current oneOf behavior (ie, oneOf and anyOf will produce identical typings) and consider it acceptable that this will cause the library to be over-accepting of cases where a oneOf schema should reject input that anyOf would accept.

This bug was discovered at the same time/for the same motivating case as #461, but is a separate root cause.

[gr2m]

I did a quick research, it seems that json-schema-to-typescript implements anyOf, at least there is a test:
https://github.com/bcherny/json-schema-to-typescript/blob/master/test/e2e/anyOf.ts

But when I run a test with the code provided by @dbjorge the result is just an union, which I understand is not correct:
https://runkit.com/gr2m/drwpow-openapi-typescript-462

export type Example =
  | {
      anyof_prop_common: "a";
      anyof_prop_a?: string;
      [k: string]: unknown;
    }
  | {
      anyof_prop_common: "b";
      anyof_prop_b?: string;
      [k: string]: unknown;
    };

[gr2m]

Or maybe it is correct? TypeScript is still a myth to me. I thought typescript unions were exclusive, but doesn't seem to be?

type HasEmail = {
    email: string
}
type HasName = {
    name: string
}

type AnyOf = HasEmail | HasName

// accepts both properties?!
const test: AnyOf = {
    name: 'test',
    email: 'yo'
}

playground

So I guess a union is the correct type here? TypeScript still confuses me ... 😕

[dbjorge]

But when I run a test with the code provided by @dbjorge the result is just an union, which I understand is not correct

I think that a union (| operator) like json-schema-to-typescript is using is the correct behavior here; openapi-typescript is currently using an intersection (& operator) instead, which is what produces the incorrect TS2322 error in the example I gave.

[gr2m]

I agree that | is better than &. We should do that first.

What I don't understand though is this claim in the linked SO answer:

type AnyOfSample = HasName | HasEmail | (HasName & HasEmail) // same thing

The above code is not the same as

type AnyOfSample = HasName | HasEmail

and would in fact be preferable. See this playground

[gr2m]

Or am I missing something?

[dbjorge]

Ah, nuts, it's me who's confused; TypeScript's names for the & and | operators are backwards from how I expected (TypeScript calls & the intersection operator and | the union operator, per https://www.typescriptlang.org/docs/handbook/unions-and-intersections.html). I'll update the issue title/etc to correct it.

No, I did have the terms right I think, nevermind this

[gr2m]

TypeScript still confuses me ... 😕

evergreen ☝️🤣

[dbjorge]

Your playground example is pretty compelling; it does feel like there's cases where just union on its own is incomplete. I'm not sure it's as simple as unionOf([...options, intersectionOf(options)]), though; I worry that it would be need to be more complex in the case where there are >2 options.

[gr2m]

it would be need to be more complex in the case where there are >2 options.

Yes, I think anyOf means a union of all items + all possible combinations of these items. So the TypeScript code would grow pretty quick, unless there is some smart hack that I'm not aware of

[gr2m]

So I was convinced by @andrewbranch and @smockle that we should do a simple union of all possible items. I'll post the whole discussion here once both agree that it's ok (@smockle already did, just wanting for @andrewbranch +1).

Here is an example to showcase that it works by Andrew: playground

The limitation is that using an object as an intersection of the union would currently fail, see the last if block in this sandbox

@andrewbranch suggested that TypeScript's current behavior of narrowing the result with in might be changed from the current never to the intersection of the union, which would be perfect for our use case - if I understand it correctly that is :D

[gr2m]

Here is the chat log for reference, lots of insights! I copied from Slack and tried to clean it up, I hope I didn't mess up anything

@andrewbranch

Some of the cognitive dissonance here comes from the fact that people typically think of TypeScript types as being closed, but they’re not. Meanwhile, I believe that OpenAPI schemas are intended to be closed. So there’s a fundamental difference between OpenAPI’s idea of oneOf and a union type in TypeScript. HasEmail and HasName are not mutually exclusive in TypeScript, but you may be able to represent that kind of mutual exclusion in OpenAPI. If they were mutually exclusive, then you would have reason to introduce another type that has both an email and a name property.

@gr2m

I understand that OpenAPI/JSON Schema & TypeScript are not compatible, they address different use cases. But it's what we got so we try to get as far with it as we can.
Could you elaborate on

HasEmail and HasName are not mutually exclusive in TypeScript

If I want to type an object that can either have an email, or a name, but not both, that's what I can do with a union, or not?
If I want to say the object must have at least one of the properties, or both, how would I express that in TypeScript?

@smockle

If I want to type an object that can either have an email, or a name, but not both, that's what I can do with a union, or not?

Like DogXorPerson in this playground?

@gr2m

yes

@smockle

This playground auto-applies the nevers, but it needs a bit more work to re-require the other values

@smockle

I don’t know how to extend Xor to take an arbitrary number of type arguments.

@smockle

Conceptually, it could be recursive: Until arity = 2, call self with the first argument + Xor of the rest

@gr2m

for my use case, I don't want an Xor. I want any possible combination of an arbitrary list of types, but at least one of them must apply. In my case the list of types are not conflicting. See sandbox

@smockle

Sorry, I was going off of this, which sounded like you wanted an xor:

If I want to type an object that can either have an email, or a name, but not both

@andrewbranch

I just typed up a very long and detailed response, and it crashed Slack. Hold pls

@andrewbranch

Oh good it saved the message.

@andrewbranch

@gr2m what I’ve been trying to say is that your OneOf type already fits what you’re describing, because like I said, the constituent types do not conflict / are not mutually exclusive. An object { email: "``[[email protected]](mailto:[email protected])``", name: "Andrew" } is already assignable to OneOf , because HasEmail does not imply the absence of name and HasName does not imply the absence of email. What you’re having trouble with is a combination of limitations and design decisions that make these two types act differently in narrowing situations, even though they are in fact equivalent from a type-theoretical perspective. What I’m trying to get at is that rather than letting these implementation details convince you to use AnyOf so you can do narrowing in this particular way, you should change the way you’re doing narrowing so that you do not need AnyOf . Although the two types are equivalent, OneOf is a much better type.

@smockle

Revisiting

gr2m

I'm trying to understand why, I'm sure you are right and I am missing something, but I don't know what

because that type is not different from HasEmail | HasName | HasId

Doesn't this sandbox prove that it's not the same?

@gr2m

so that you do not need AnyOf

I do not have control over the types. I talked to the OpenAPI folks (@xuorig) and there are cases were an AnyOf schema cannot be avoided.

the root of the issue is the conditions in your if aren’t testing what you think they’re testing

Can you ellaborate **@smockle? With the typeguards I'm avoing TypeScript errors. If I left them out and would just do**

assertString(testAnyOf.name)
assertString(testAnyOf.email)

In my code, TypeScript would not compile

@gr2m

I really appreciate your patience with me here

@andrewbranch

I’m talking about your AnyOf type, not the usage of anyOf in the OpenAPI schema.

@smockle

Example

@andrewbranch

**What the example you mentioned earlier is proving is that our narrowing logic is imperfect. If you try to narrow a discriminated union, we will only ever filter the existing union down to a smaller one. So if you try to apply your double in-narrowing to my suggestion to use only OneOf, you hit this problem where testOneOf narrows to never . Follow me so far @gr2m**
?

@smockle

Right, it’s not positive assertions about the contents of the object, it’s negative assertions about members of the type set

@andrewbranch

What’s happening here is we are pretending, incorrectly, that there is no object that fits HasEmail | HasName that has both name and email properties. I have been repeatedly claiming that this is not true, but it would be very natural to be confused because it really seems like the compiler is telling you that you could not have a valid OneOf inside this if statement body.

@andrewbranch

Because of the way we do union narrowing in general, combined with the particular unsound way we do in narrowing, we are giving you a result here that is just plain wrong.

@gr2m

with "we" you mean the TypeScript compiler here?

@andrewbranch

Yep

@gr2m

Okay so my example is wrong.

@andrewbranch

So you need to work around a TypeScript design limitation. Broadly, you can do this in two ways:

1. You could define a very large and type-theoretically meaningless union type that explicitly includes the power set / cross product of intersections of its constituents, which is what you have with your AnyOf type.
2. You could use a different kind of narrowing that doesn’t trigger this bug.

@andrewbranch

My suggestion is to use the second approach, namely by using user-defined type guards, like this

@smockle

@andrewbranch you’re saying the narrowing works like this: initially testOneOf is either HasEmail or HasName. The TypeScript compiler wants to narrow from this set of two possible types. Applying the condition "name" in testOneOf, the compiler removes HasEmail from consideration, because there is no "name" in HasEmail so HasName is the remaining type in the set. Applying the condition "email" in testOneOf, the compiler removes HasName from consideration, because there is no "email" in HasName, so there are no remaining items in the set of possible types for testOneOf. So, the compiler is left with never?

@andrewbranch

Exactly

@andrewbranch

And honestly, digging into it this deeply, I’m not 100% sure why we don’t hit the same problem when we combine the two type guards. But for whatever reason, instead of saying “well, we can’t filter down any more, so we have the empty set,” we take the intersection of the types we have tested.

@andrewbranch

As it turns out, taking the intersection of types you’ve narrowed to is always correct. But we don’t do that all the time because when you take intersections of unions, you get much bigger unions. So usually if you have a union type, what you’re trying to do is narrow to one or more constituents already in that union. So under many circumstances we pretend that the value could not be anything not already represented in that union. This yields better performance, and often a better UX, but is not strictly correct.

@gr2m

I'm trying to put myself into the shoes of an Octokit user. I'm kinda the middleman creating the library. I do not control the types, they come from GitHub's REST API specification. And I don't control how my library is used. It will be on them to use way of typeguarding. I just want to make sure that our types permit all possible values that can be passed into our methods.
There is code like this (simplified example)

octokit.users.updateUser(options)

The type for option is directly coming from the OpenAPI spec. It could be the anyOf with email or name or email + name
So all these should be valid

octokit.users.updateUser({ email })
octokit.users.updateUser({ name })
octokit.users.updateUser({ email, name })

But this should not be valid

octokit.users.updateUser({})

In reality the types are more complicated. E.g. for updating a check run: https://gist.github.com/gr2m/2fddfcdbcb3f6ba983dc705d57ac2bfb
What is the options type for octokit.users.updateUser(options)

type options = HasName | HasEmail would fail for
octokit.users.updateUser({ email, name })

wouldn't it?
(sorry I gotta take care of the kids, i'll be back later)

@smockle

I believe that’s lines 14–17 of the example I shared above

@andrewbranch

Yeah, so on that side it would fail due to excess property checking

Let me see if I can come up with a pattern that would work well for that.
Edit, maybe I’m wrong with Clay’s example

@andrewbranch

Yeah, that seems good

@andrewbranch

As a follow up, I’m curious if we can make that in narrowing result in the intersection instead of never . Maybe that’s a bug we could fix

@gr2m

Thanks! I'll test if that works with the more complex scenarios

[drwpow]

Is this resolved in #491? Or is something additional needed?

[gr2m]

I think this is resolved as good as it can be

[kgtkr]

I'm sorry to reply to the closed issue.

TypeScript union (|): Not exclusive
JSON Schema anyOf: Not exclusive
JSON Schema oneOf: exclusive

So anyOf should be encoded into union.
oneOf is exclusive, but it's difficult to express this in TypeScript. So I think it's a good compromise to express it in union.