Allow usage for indexed types on body elements

[mahirk]

Is your feature request related to a problem? Please describe.
The JSON specification allows an indexed type by defining it as an object which can have any number of strings in it, for example:
the following type definition:

metadata?: {
        [key: string]: string;
    };

Becomes:

        "metadata": {
          "additionalProperties": {
            "type": "string"
          },
          "type": "object"
        },

Describe the solution you'd like
The following spec fails:

class Api {}

@endpoint({
  method: "POST",
  path: "/users/:someOtherString"
})
class CreateUser {
  @request
  request(
    @headers
    headers: HeaderAlias,
    @pathParams
    pathParams: PathParams,
    @body body: CreateUserRequest
  ) {}

  @response({ status: 201 })
  successResponse(@body body: CreateUserResponse) {}
}


interface CreateUserResponse {
  firstName: string;
  lastName: string;
  role: string;
  [key: string]: string
}

with the error as indexed types are not allowed.

The proposal would be to update the type-parser to allow indexed types and define them using the guidelines provided above.

[mahirk]

@lfportal Happy to work on it, and get any implementation ideas you may have on the same. Any motivation on why it was initially not permitted would be beneficial!

[mahirk]

I also noticed the following which capability matrix:
union type -> ✅
inheritance type -> ✅
recursive -> 🔴
generics -> 🔴

Would love to hear any thoughts you have on implementing these.

[lfportal]

Any motivation on why it was initially not permitted would be beneficial!

This was considered, but was not a priority use case for us. Ultimately we deemed TypeScript's indexed types not flexible enough to support this feature in a sufficiently generic way. Where an indexed type is present, defined sibling properties must also be assignable to the index type definition:

interface CreateUserResponse {
  firstName: string; // Property 'firstName' of type 'string' is not assignable to string index type 'boolean'.
  [key: string]: boolean;
}

[lfportal]

recursive -> 🔴

The type parser would need to be modified to pass around some sort "type parse stack" structure to keep track of types currently being parsed as it traverses the types to short circuit reference type parsing. I'm not too familiar with recursive type representation with the OpenAPI spec. Are you aware of any particular limitations @mahirk?

generics -> 🔴

interface Generic<T> {
  genericProp: T;
}

type A = Generic<String>

Off the top of my head:

1. Parse type parameters (T)
2. Parse the generic interface, passing in the resolved type parameters

Questions that need answering:

• Do we store the resulting type in the typeTable?
  • I'm thinking we don't, and treat it as if it were defined inline (i.e. type A = { genericProp: String; }) to keep it simple
  • If we do, the need some way to name the type. GenericString, GenericInt? But this could lead to type name clashes

Thoughts @mahirk?

[mahirk]

Apologies, @lfportal got pulled off to something else:

Ultimately we deemed TypeScript's indexed types not flexible enough to support this feature in a sufficiently generic way

Absolutely, I am curious though, what about scenarios which could enable an open-ended pair of single level JSONs? Although I do understand the type safety aspect of it, I was thinking more along the lines of the metadata which is available here: https://stripe.com/docs/api/metadata. You can see the definition here: https://raw.github.com/stripe/openapi/master/openapi/spec3.yaml

metadata:
          additionalProperties:
            maxLength: 500
            type: string
          description: Set of [key-value pairs](https://stripe.com/docs/api/metadata)
            that you can attach to an object. This can be useful for storing additional
            information about the object in a structured format.
          type: object

recursive -> 🔴 ....Are you aware of any particular limitations @mahirk?

Swagger UI seems to have trouble, and potentially the model generation might have issues. Let me check those and then evaluate if it would be okay. As far as representation, that's possible using the specification swagger-api/swagger-ui#3325

Questions that need answering

I agree with that, it may be something the system calls out to the developers. I don't forsee any issues with generating the type in that way however.

[lfportal]

Absolutely, I am curious though, what about scenarios which could enable an open-ended pair of single level JSONs?

Do you mean to allow index signatures only where no sibling properties are defined? i.e:

// allowed
interface A { 
  [key: string]: string;
}

// not allowed
interface B {
  [key: string]: string;
  a: string
}

[mahirk]

Yup @lfportal i.e. if it has sibling properties, then spot can log an error stating that sibling properties on index types are not permitted. That should satisfy the use case and maybe also force better design of open-ended structs?

[lfportal]

I'm thinking a simpler way to constrain the usage could be TypeScript's Record<string, *> type. The parser would only need to constrain the first type argument to string - disallowing number and literals "a" | "b" | "c".

[Noeyfan]

Hi @mahirk @lfportal

Catching up with the discussion.

generics -> 🔴

interface Generic<T> {
  genericProp: T;
}

type A = Generic<String>

Off the top of my head:

1. Parse type parameters (T)
2. Parse the generic interface, passing in the resolved type parameters

Questions that need answering:

• Do we store the resulting type in the typeTable?

Just to confirm we will store Generic in the typeTable regardless of we storing the resulting types, right?

• I'm thinking we don't, and treat it as if it were defined inline (i.e. type A = { genericProp: String; }) to keep it simple
• If we do, the need some way to name the type. GenericString, GenericInt? But this could lead to type name clashes

inline type works great when the Generic object is small, but when its big and frequently referenced, the cost of expanding it every where goes up very quickly. The name clashes should not be a problem if we just use the full name Generic<String>. Maybe we can have a threshold on when we should put it into typeTable?

Also, are we only considering handle custom generic type, or we will also support typescript utility types? https://www.typescriptlang.org/docs/handbook/utility-types.html

As a side note on typescript supported types, I noticed: undefined, object, unknown are giving unknown type error at the moment, is it expected?

generics/recursive/index-type/util-type It feels like we should create few separate issue to address each of the problem and discuss them separately? thoughts?

[lfportal]

inline type works great when the Generic object is small, but when its big and frequently referenced, the cost of expanding it every where goes up very quickly. The name clashes should not be a problem if we just use the full name Generic<String>

The type table is used to generate the components section of OpenAPI documents. We would require some strategy to convert Generic<String> into something OpenAPI friendly. Whatever that becomes will be subject to name clashing with user defined types (this will be true for typescript's utility types too). I think we should aim to avoid any implicit naming, and stick with a clear and predictable syntax. Currently the type table only stores types declared using type aliases and interfaces. I don't see any need to complicate this:

body: {
  a: Generic<String>; // inline type, rendered as an inline schema in OpenAPI
  b: MyType; // reference type, rendered as a $ref schema in OpenAPI
}

type MyType = Generic<String> // stored in type table

As a side note on typescript supported types, I noticed: undefined, object, unknown are giving unknown type error at the moment, is it expected?

This is currently expected. However, undefined is indirectly supported using the question token to indicate optionality myOptionalField?: string. Do you have a use case for using these types? Perhaps we can discuss them in a separate issue/s.

generics/recursive/index-type/util-type It feels like we should create few separate issue to address each of the problem and discuss them separately? thoughts?

Yup, agreed, I'll leave this issue to focus on indexed types.

[jankuca]

This feature would be really useful for us as well. Allowing indexed types without explicitly listed siblings is more than enough.