Additional properties map to empty interfaces (OpenAPI v3)

[eysi09]

Hi!

Thinks for a great library, we use it quite a lot for generating API types for our frontend, based on the backend spec.

However, I'm having issues with additionalProperties (using the OpenAPI v3 spec) mapping to empty interfaces.

As an example from our source code, we have the following Typescript interfaces:

export type Primitive = string | number | boolean | null

export interface PrimitiveMap {
  [key: string]: Primitive
}

We use tsoa to generate an OpenAPI v3 spec which looks something like this:

"Primitive": {
  "anyOf": [
    {
      "type": "string"
    },
    {
      "type": "number",
      "format": "double"
    },
    {
      "type": "boolean"
    },
    {
      "type": "number",
      "enum": [
        null
      ],
      "nullable": true
    }
  ]
},
"PrimitiveMap": {
  "properties": {},
  "type": "object",
  "additionalProperties": {
    "$ref": "#/components/schemas/Primitive"
  }
},

AFAICT, the spec looks right. But when I generate the types with swagger-typescript-api I get the following results:

export type Primitive = string | number | boolean | "null" | (string & number & boolean & "null");

export interface PrimitiveMap {}

Notice how the PrimitiveMap is empty.

Instead, I would have expected:

export type Primitive = string | number | boolean | "null" | (string & number & boolean & "null");

export interface PrimitiveMap {
  [key: string]: Primitive
}

All help would be much appreciated!

[eysi09]

Ok, so digging a little deeper into this. The culprit seems to be the empty "properties" field on the "PrimitiveMap". It works as expected if I remove it. But I don't believe I can do that since the spec is auto-generated.

[eysi09]

Looks like changing this line: https://github.com/acacode/swagger-typescript-api/blob/master/src/schema.js#L49 to:

if (!_.isEmpty(schema.properties)) return "object";

does the trick.

I'd be happy to submit a PR if this makes sense. I also wonder if enums in the line above should be treated the same.

[js2me]

@eysi09 Thanks for this investigation of the source code :)
I have plans to fix it today (Moscow time)

Also, thanks for this quick-fix solution, I'll check it and maybe do the same as you offered.

[js2me]

I fixed that in the next branch. Thanks for help @eysi09

[js2me]

It should be fixed in 1.12.0

[eysi09]

Thanks for the quick response and fix @js2me!

Could I furthermore also ask for your feedback on one thing? The type above gets transformed as follows:

// Original
export interface PrimitiveMap {
  [key: string]: Primitive
}

// Map to Swagger

// Convert from Swagger via swagger-typescript-api
export type PrimitiveMap = Record<string, Primitive>

However, we also have a type called DeepPrimitiveMap which is recursive and looks like this:

export interface DeepPrimitiveMap {
  [key: string]: Primitive | DeepPrimitiveMap | Primitive[] | DeepPrimitiveMap[]
}

Because of the recursiveness, it can't be mapped to a Record and the compiler throws.

As I was playing around with this, I noticed I could get the types I needed by modifying your code like so:

  primitive: (schema, typeName) => {
    let contentType = null;
    let typeIdentifier = "type";
    const { additionalProperties, type, description } = schema || {};

    if (type === "object" && _.isObject(additionalProperties)) {
      const fieldType = getInlineParseContent(additionalProperties);
      typeIdentifier = "interface"; // <---- Set type identifier to interface
      contentType = `[key: string]: ${fieldType}`; // <---- use [key: string] instead of Record
    }

    if (_.isArray(type) && type.length) {
      contentType = complexSchemaParsers.oneOf({
        oneOf: type.map((type) => ({ type })),
        description,
      });
    }

    return attachParsedRef(schema, {
      $parsedSchema: true,
      schemaType: "primitive",
      type: "primitive",
      typeIdentifier,
      name: typeName,
      description: formatDescription(description),
      // TODO: probably it should be refactored. `type === 'null'` is not flexible
      content: type === "null" ? type : contentType || getType(schema),
    });
  },

Now my DeepPrimitiveMap is transformed to:

export interface DeepPrimitiveMap {
  [key: string]: Primitive | DeepPrimitiveMap | Primitive[] | DeepPrimitiveMap[]
}

So I guess my question is, would it make sense for this library to handle this case, perhaps as an optional flag? Or is using a Record somehow more correct and there are edge cases I'm not seeing?

Of course flat out changing this would be a breaking change so I'm not suggesting that.

[js2me]

Hello @eysi09 , open PR with this fix if you can
Also you can check test schemas with script test:all