Property without type should be declared as unknown

[ggrossetie]

Description

Name	Version
openapi-typescript	6.1.1
Node.js	18.12.2
OS + version	Ubuntu 22.04.2

Reproduction

1. Create a file named api.json with the following content:

api.json

{
  "openapi": "3.0.0",
  "components": {
    "schemas": {
      "User": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "Info": {
            "nullable": false
          }
        }
      }
    }
  }
}

2. Run openapi-typescript:

$ npx openapi-typescript api.json --output type.ts
✨ openapi-typescript 6.1.1
🚀 api.json → file:///path/to/type.ts [12ms]

3. You should get the following result:

type.ts

/**
 * This file was auto-generated by openapi-typescript.
 * Do not make direct changes to the file.
 */


export type paths = Record<string, never>;

export type webhooks = Record<string, never>;

export interface components {
  schemas: {
    User: {
      Info?: Record<string, never>;
    };
  };
  responses: never;
  parameters: never;
  requestBodies: never;
  headers: never;
  pathItems: never;
}

export type external = Record<string, never>;

export type operations = Record<string, never>;

As you can see, schemas.User.Info is defined as Record<string, never> which is incorrect because Info might contain a string, number or boolean and not necessarily an object.

Expected result

export interface components {
  schemas: {
    User: {
      Info?: unknown;
    };
  };
  responses: never;
  parameters: never;
  requestBodies: never;
  headers: never;
  pathItems: never;
}

Checklist

• I’m willing to open a PR (see CONTRIBUTING.md)

[ggrossetie]

It might be the same as #1031 and might be resolved by #1032

@duncanbeevers could you please confirm (or not) that this is the same issue?

[duncanbeevers]

Yes, I can confirm that #1032 would resolve this issue.

The type of Info is still a Record, which is a little odd; as you pointed-out @ggrossetie, Info could be a string, a boolean, a number, etc…

I think your proposal that Info should be typed as unknown is correct.

[ggrossetie]

Thanks!
I will keep this issue open since it goes a bit further and suggest to use unknown instead of Record<string, unknown> when the type is unknown.

[drwpow]

I believe this is taken care of by the --empty-objects-unknown flag introduced in #1032. I’m still somewhat against loosening up the output types to unknown or even Record<string, unknown> as default behavior (explanation).

I also don’t want to debate this topic too much more, because I think no matter how you answer the question “what types should be generated when my schema doesn’t specify a type” isn’t a very productive conversation. No matter what the decision is, the fact is that your schema is lacking critical information needed to type it safely. And I’d rather err on the side of generating types to encourage you to fix it at the schema level, not the typegen level.

[duncanbeevers]

And I’d rather err on the side of generating types to encourage you to fix it at the schema level, not the typegen level.

I agree with this stance. Strict-by-default guides API designers towards better schemas.

Importantly though, this tool is used to generate types for APIs outside of one's own control, so having controls for loosening the generated types is key.

[mitchell-merry]

Importantly though, this tool is used to generate types for APIs outside of one's own control, so having controls for loosening the generated types is key.

Exactly.

Besides, I don't see why a discussion is necessary, since the spec is well-defined for these cases as I've demonstrated. Specs of this form - without type specified - are perfectly legal according to the spec, but you're choosing here to not support them correctly. Why? What if we genuinely want to accept an unknown type? (that is, any value? number, string, boolean, array, object, whatever)

Realistically, if this doesn't make it into project, I'm just going to have to complicate generation on my side as I'll need to use the Node API and specify a custom transform, and then implement this logic myself.

[drwpow]

@mitchell-merry Sorry—just for clarification, is the --empty-objects-unknown flag not generating what you’d like? It would be helpful for me to clearly see the differences between a) the default generation, b) --empty-objects-unknown, and c) what your proposal is. Just so we’re all on the same page.

[drwpow]

Oh, sorry—I understand now. Yes that doesn’t conflict and this should be fixed (rubber ducking works!). I’ll re-open your PR.

[ggrossetie]

It would be helpful for me to clearly see the differences between a) the default generation, b) --empty-objects-unknown, and c) what your proposal is. Just so we’re all on the same page.

Sure, I will do that tomorrow but I believe it's:

a)

Record<string, never>

b)

Record<string, unknown>

c)

unknown

(as mentioned by @duncanbeevers in #1039 (comment))
But I will confirm tomorrow using the latest release.

Oh, sorry—I understand now. Yes that doesn’t conflict and this should be fixed (rubber ducking works!). I’ll re-open your PR.

No worries 😉