Nullable fields not respected in intercom schema?

[cipriancaba]

Description

This is my first time using this library, it's absolutely amazing, thank you so much 🙏

I'm having a bit of trouble with the | undefined parts of the generated types. It seems that absolutely everything is | undefined For example, in this call to GET /tags, it correctly discriminates the non error nature of the response, but the generated type is:

tags.data.data is possibly 
 .
(property) data?: {
    [key: string]: unknown;
    type?: string | undefined;
    id?: string | undefined;
    name?: string | undefined;
    applied_at?: number | undefined;
    applied_by?: {
        ...;
    } | undefined;
}[] | undefined

And here is my code

const tags = await intercomClient.GET('/tags')
  if (tags.error) {
    return NextResponse.json(tags.error)
  }

  for (const tag of tags.data.data) { //<-- errors here as data array can be undefined
    console.log(tag)
  }

Been playing around with the --default-non-nullable but it doesn't seem to do the trick. I'm sure it's an obvious problem, but I can't figure it out.

Thanks for the help

Name	Version
openapi-typescript	7.0.0-next.2
Node.js	18.20
OS + version	macOS Sonoma

Reproduction

https://gist.github.com/cipriancaba/790b7366794640779f0ca662f759ef8e

npx openapi-typescript@next intercom.yaml -o intercom.d.ts

Expected result

Generated data type shouldn't be | undefined, unless nullable is specified in the yaml schema eg:

(property) data?: {
    [key: string]: unknown;
    type?: string;
    id?: string;
    name?: string;
    applied_at?: number;
    applied_by?: {
        ...;
    };
}[]

Checklist

• My OpenAPI schema passes the Redocly validator (npx @redocly/cli@latest lint)
• I’m willing to open a PR (see CONTRIBUTING.md)

[drwpow]

Taking a quick glance at your schema, I think it’s because you have nullable: true set on many schema objects, as well as missing a lot of required arrays. At first glance it seems to be generating the types as-expected, but if there’s a specific example that seems weird I’d be happy to take a closer look.

[drwpow]

Also --default-non-nullable is for something different. That’s for times when you are setting a default key to treat it as if the property is required even if it’s not explicitly set. In v7 and beyond this defaults to true; the only reason it exists still is for backwards compatibility with the previous behavior (e.g. --default-non-nullable=false)

[cipriancaba]

Hey, thanks so much for being so helpful. I've created a repo showing my problem here https://github.com/cipriancaba/openapi-typescript

This is the schema for the specific GET /tags request

tag:
      title: Tag
      type: object
      x-tags:
      - Tags
      description: A tag allows you to label your contacts, companies, and conversations
        and list them using that tag.
      properties:
        type:
          type: string
          description: value is "tag"
          example: tag
        id:
          type: string
          description: The id of the tag
          example: '123456'
        name:
          type: string
          description: The name of the tag
          example: Test tag
        applied_at:
          type: integer
          format: date-time
          description: The time when the tag was applied to the object
          example: 1663597223
        applied_by:
          "$ref": "#/components/schemas/reference"

I can see indeed it is missing the required property as true. Is the default expectation that a missing require is actually false? If so, is there a way to toggle that and maybe just consider a field nullable if nullable is set to true in the schema?

[drwpow]

Is the default expectation that a missing require is actually false?

Yes that’s the behavior that’s consistent in OpenAPI 3 (and 2) and JSONSchema (there are only a couple exceptions, such as path params always being required but that’s because by their nature they must be). Since this behavior is so common there’s no option to override it. Because you likely won’t have compatibility with other tools that enforce this same behavior. Best practice recommends following the spec and treating everything as optional unless explicitly required.

[cipriancaba]

I see, that would be amazing if I actually had control over the schema. The problem seems to be with more of the schemas I am using integrating different services.

Do you think it might be helpful to add a flag that treats non-required fields as required if the nullable is present? If you're open to the idea and point me in the right direction, would be more than happy to open a PR

[drwpow]

I’d rather not add a flag to support this because it would encourage going against using OpenAPI as it’s designed, and how it’s worked for a long time. The real fix here is using valid schemas that adhere to the OpenAPI specification as-written. But I understand that’s beyond your control.

Because this is a unique scenario that few people encounter, I’d recommend using the transform API to override generated types based on the schema logic. Or you could simply fork the API schema and make the changes yourself (which for a public API that can’t introduce breaking changes, you won’t have to update this very much if at all).

[cipriancaba]

Thanks again @drwpow . I totally understand your reasoning, however this api is just one of those that we use, and keeping updated copies of every single one is not going to work in the long run.

I've gave the transform API a shot, but not sure how I can access the default transform, I am trying to achieve something like this:

import fs from 'node:fs'
import openapiTS from 'openapi-typescript'

const localPath = new URL('./intercom.yaml', import.meta.url) // may be YAML or JSON format
const output = await openapiTS(localPath, {
  transform(schemaObject, metadata) {
    if (!('nullable' in schemaObject)) {
      return defaultTransform({ ...schemaObject, required: true }, metadata)
    }
  },
})

fs.writeFileSync('intercom.d.ts', output)

I don't want to rewrite the entire transformer, just update the schemaObject to actually include required: true and pass it back to the defaultTransformer. I see how this might cause an infinite loop, and this is just a naive example, but not sure how else I would go about this? I guess I am looking for a middleware more than a transform?

[drwpow]

I see how this might cause an infinite loop

Oh shoot yeah that’s just a bad example; will fix that. It’s more meant to represent “if a node meets condition A then I want to override the type; otherwise, transform as normal.” But you could just return literal null to not override the default.

Also in the current version (6.x), you can use postTransform() to deal with strings, which is both simpler and more difficult (simpler to see that the type is, but harder to string mash more complicated types). In the upcoming version (7.x), transform/postTransform deal with the TypeScript AST, so manipulation is much easier.

I guess I am looking for a middleware more than a transform?

Yes that sounds like it. That would be outside the purpose of this library, as this only translates a valid schema into TS and won’t give you any tools to modify your schema. I’d look at Redocly—this sounds like you’d want to create a plugin to modify your schema on-the-fly (also fun fact: the upcoming 7.x version bundles Redocly core for validation & extra features).

[cipriancaba]

I am on version 7 currently, figured I'd get a head start on it anyway :)

Let me see if I got this clear, in the transform function, you are passing the schemaObject and the metadata, but if I want to access the output of the default transformer, there is no way for me to do that?

Which basically means that there is no way to update the result of the transform (remove the | undefined part) on a node that's missing the required property? Unless I use redocly to transform the actual schema which is missing the required props and then pass that to openapi-typescript?

Is there a way to access the redocly plugin directly from openapi-typescript in v7?

[drwpow]

Basically you’re modifying your schema—you are changing the contract that the API server has with its consumers. This is in “bad practice” territory and is nonstandard (I know you don’t have control, but the root issue is an unreliable schema, and the real fix).

I’ve been suggesting “hacks” in openapi-typescript as a shortcut for you if it’s simple enough to work, but you should treat this problem as what it is—a consumer modifying the original schema ad-hoc. And there’s no standard support or happy path for that. But if you are in that territory, I’d suggest looking into Redocly’s plugins as it’s built exactly for that purpose, and I’m confident that approach will last (and 7.x isn’t quite ready yet, so even if using Redocly in a specific way in 7.x works in the beta version don’t take that as a guarantee it will in the stable release).

[cipriancaba]

Thanks @drwpow, really appreciate the help and thanks for the amazing library

[drwpow]

Yeah it’s a tricky problem and not one many people face! But again, Redocly almost certainly has the suite of tools you need if you run it before openapi-typescript. Good luck! 🙂