hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
Other
1.07k stars 89 forks source link

Node API: allow transforming types to append null into optional fields #549

Open dagadbm opened 4 months ago

dagadbm commented 4 months ago

Description

So we have some APIs that have optional fields, or fields that may not have values, these are usually sent over the wire in JSON and they have null as their value. The problem here is that the openapi generated types dont add a .. | null to these types and we end up in a situation were the types dont really represent the reality of the API call, which means our app code needs to change and our mock data for tests also need to change to conform to these types.

It would be very helpful if this could be fixed or at least provide some option for adding | null to these optional types.

Here is a very short example of what I mean. We are using fastapi in the back-end with pydantic model that gets converted and then generates the openAPI spec.

This is the model:

class ApiInstallationSettings(pydantic.BaseModel):
    mail_default_sender: str | None

It generates this openAPI spec:

      "ApiInstallationSettings": {
        "properties": {
          "mail_default_sender": {
            "title": "Mail Default Sender",
            "type": "string"
          },
        },
        "required": [
        ],
        "title": "ApiInstallationSettings",
        "type": "object"
      },
      "ApiInstallationSettings": {
        "properties": {
          "mail_default_sender": {
            "title": "Mail Default Sender",
            "type": "string"
          },
        },
        "required": [
        ],
        "title": "ApiInstallationSettings",
        "type": "object"
      }

    "/api/internal/settings/installation": {
      "get": {
        "operationId": "get_install_settings_api_internal_settings_installation_get",
        "responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/ApiInstallationSettings"
                }
              }
            },
            "description": "Successful Response"
          }
        },
        "summary": "Get Install Settings",
        "tags": [
          "Settings"
        ]
      },

And the generate types using hey is this:

export type ApiInstallationSettings = {
  mail_default_sender?: string;
};

However, this is **wrong*** (at least for our use case) because as you know json doesnt really have undefined and in the cases when this field is actually missing what we receive over the wire is null and not undefined. If the field was indeed missing at all you wouldn't even get this field at all in the payload.

Is there an easy way to update the configuration to handle these optional types in a better way? at the very least add a mail_default_sender?: string | null instead of only mail_default_sender?: string.

This ends up causing all sorts of typing problems on our side specially on the unit tests and sometimes on the app logic as well as indeed we do receive nulls for when the values dont exist and not really undefined.

Configuration

openapi-ts --input ./backend/api/openapi.json --output ./frontend/app/rest/ --useOptions --client axios --postfixServices Controller --base=''"

dagadbm commented 4 months ago

I would also like to add it is amazing to see this project being lifted and growing as it has been. I know open source is very hard to do and I don't want to seem ungrateful or demanding so I believe I should add this extra comment here just saying thank you 🙇 as it is the least I can do.

mrlubos commented 4 months ago

Hey @dagadbm, thank you for the kind words! Now onto the problem. If you look at your spec, there really isn't anything to indicate that a field is nullable. You'd want to see the nullable: true field or null in the type array. Without digging deeper into it, this might be relevant? https://github.com/tiangolo/fastapi/discussions/6910

Alternatively, say you didn't have control over your spec, and wanted to add this functionality. Instead of making this a config option, I'd prefer to give you a Node API where you could transform any field however you please.

What are your thoughts?

dagadbm commented 4 months ago

i believe it could be a good compromise i'm just unsure on how to use it afterwards.

I will take a look at the fastapi issue but we have so many fields like this i dont think it will be feasible to do any sort of big change there