hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
MIT License
748 stars 60 forks source link

Restore old openapi-typescript-codegen enum behaviour #557

Open alessiomatricardi opened 2 months ago

alessiomatricardi commented 2 months ago

Description

I'm attepting to migrate from openapi-typescript-codegen to this new library (which seems very complete, bravo!) But I feel that new enums working structure is not complete as the old one.

With the old library, I was able to have enums exported with type namespace eg image While now it appears as below image I lost the ability of write Order.status.PLACED which was referring to placed value

Also, I don't notice any difference between "javascipt" and "typescript" value for types.enums

OpenAPI specification (optional)

https://petstore3.swagger.io/api/v3/openapi.json

Configuration

OLD openapi -i /home/amatricardi/Downloads/openapi.json -o src/models/pets --exportCore false --exportServices false --indent 2

NEW openapi-ts -i /home/amatricardi/Downloads/openapi.json -o src/models/pets2 --exportCore false --services false --schemas false --format prettier --lint eslint

System information (optional)

No response

mrlubos commented 2 months ago

Hey @alessiomatricardi, would you be okay with having the status enum exported on its own without namespace?

alessiomatricardi commented 2 months ago

@mrlubos could be a good compromise :) If it is not too hard to implement, I would do a try also with namespaces, maybe as option of types.enums. Let me know if it can be a good idea :)

OT Is there a way to have multiple configurations inside the defineConfig? I have multiple OpenAPI definitions, and need to generate all of them optionally, maybe calling openapi-ts <config-name> where defineConfig takes {[k: string] : UserConfig}

mrlubos commented 2 months ago

Can you describe why you'd want to try with namespaces?

There's currently no way to have multiple configurations, but you're not the first one to ask about it. https://github.com/hey-api/openapi-ts/discussions/444

alessiomatricardi commented 2 months ago

Can you describe why you'd want to try with namespaces?

Nothing really important, just for backward compatibility, but I'm ok with simple exported enums 👍🏼

There's currently no way to have multiple configurations, but you're not the first one to ask about it. #444

Sorry I didn't notice about that issue. Could a workspace (as suggested here https://github.com/hey-api/openapi-ts/discussions/444#discussioncomment-9165006) be an option for the future development of the library?

Anyway, thanks a lot :)

mrlubos commented 2 months ago

Potentially 😀 I haven't explored it yet. There's a long list of feature requests before that!

mrclrchtr commented 2 months ago

I also wanted to migrate from openapi-typescript-codegen today and encountered exactly the same problem.

Also, I don't notice any difference between "javascipt" and "typescript" value for types.enums

It doesn't seem to make any difference whether I specify

types: {
    enums: "javascript",
  },

or not. The code is identical.

mrlubos commented 2 months ago

@mrclrchtr Can you check with the latest release (v0.45.0) and let me know if it's any better?

mrclrchtr commented 2 months ago

@mrlubos, it's working now as expected. Thank you very much!

mrlubos commented 2 months ago

Thanks @mrclrchtr! How about you @alessiomatricardi?

alessiomatricardi commented 2 months ago

Hi, sorry for late reply. I'm currently very busy with work, will come back to you asap :)

elibolonur commented 1 month ago

Hi there, first of thank you very much for this beautiful package!

To raise up another problem related to namespaced enums; We had something like this in the old package:

export type FieldResponse = {
  id: string;
  name: string;
  /**
   * Type of this field
   */
  type: FieldResponse.type;
};

export namespace FieldResponse {
  /**
   * Type of this column.
   */
  export enum type {
    NUMERICAL = 'numerical',
    BOOLEAN = 'boolean',
    TEXT = 'text',
    DATE = 'date',
    ANY = 'any',
  }
}

Which the schema part looks like this:

"FieldResponse": {
  "title": "row",
  "type": "object",
  "properties": {
    "id": {
      "title": "id",
      "type": "string"
    },
    "name": {
      "title": "Name",
      "type": "string"
    },
    "type": {
      "title": "Type",
      "description": "Type of this field",
      "enum": [
        "numerical",
        "boolean",
        "text",
        "date",
        "any"
      ],
      "type": "string"
    },
  },
  "required": [
    "id",
    "name",
    "type"
  ]
}

Now when I generate with hey-api with following:

// config
{
  ...
  services: {
    asClass: true,
  },
  types: {
    enums: 'javascript',
  }
  ...
}

Inline types are generated outside of the namespace, which looks nice at first. However, if they are conflicting with other names (that there are other namespaced enums with the same prop name) enum name becomes a little bit cryptic:

export type FieldResponse = {
  id: string
  name: string
  type: 'numerical' | 'boolean' | 'text' | 'date' | 'any'
}

/**
 * Type of this column.
 */
export type type22 = 'numerical' | 'boolean' | 'text' | 'date' | 'any'

// or simply the typescript enums version:
export enum type22 {
  NUMERICAL = 'numerical',
  BOOLEAN = 'boolean',
  TEXT = 'text',
  DATE = 'date',
  ANY = 'any',
}

Reason is as mentioned, that there are simply other attributes named same as type, and which normally should be namespaced. I know that namespaces generate IIFEs and other tree shaking problems that has been discussed in the other issues, however imho this should be an option in the configuration to give a choice to the user. If namespacing is an absolute no go, then we could maybe consider prefixing the const name with the parent type name e.g. const FieldResponse_type.

Otherwise it is not preferable to use enums like: if (fieldItem.type === type22.NUMERICAL) kind of cryptic approach, likely a dealbreaker when asking to the team to migrate.

Hope it is helpful, let me know if I can help! @mrlubos

swaroopar commented 1 month ago

@mrlubos Thank you as always for the great library and the efforts.

Would it be possible to generate one single enum and then refer it in all types that uses the same enum? Currently with enum: typescript, we are generating an enum and then additionally one string literal inside each type generated.

P.S - we also generate java client from openapi files using a similar library and there we get just one enum and it is referred from all other types. This makes using the enum very neat.

mrlubos commented 1 month ago

@swaroopar yes, that's the dream 😀 one day

elibolonur commented 1 week ago

@mrlubos is there any updates with this?

mrlubos commented 1 week ago

@elibolonur Not on the single enum reference