openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

openapi-fetch: Swagger v2 support

Open Sawtaytoes opened this issue 1 year ago • 8 comments

Description

When making a query, everything's typed and working properly, but when I examine the data and error props, I get undefined even though the 200 value is clearly defined in the generated .ts file.

Reproduction

I'm using the SalesIntel API: https://api.salesintel.io/.

Since it's running on Swagger v2, I'm using openapi-typescript v5 to generate the .ts file. This could be the issue, but I don't have any context on what's changed between v5 and v6 and which openapi-fetch I should be using.

Node.js v18.15.0:

"openapi-fetch": "^0.6.0",
"openapi-typescript": "^5.4.1",
"typescript": "^5.1.6"

Here's the endpoint:

export interface definitions {
  NewsResponseDto: {
    aggregations?: definitions["JsonNode"];
    batch_id?: definitions["JsonNode"];
    error?: string;
    is_preview?: boolean;
    /** Format: int32 */
    page?: number;
    /** Format: int32 */
    page_size?: number;
    /** Format: int32 */
    result_count?: number;
    search_results?: definitions["NewsSearchResults"][];
    sort_by?: string;
    sort_direction?: string;
    /** Format: int64 */
    took?: number;
    /** Format: int32 */
    total_count?: number;
    tracking_id?: string;
  };
};

export interface paths {
  "/service/news": {
    get: {
      parameters: {
        query: {
          /** List of domains separated by comma */
          company_domains: string;
          /** Extracted date in format (yyyyMMdd - 2023-04-01) to return news from */
          extracted_begin_date?: string;
          /** List of news categories separated by comma */
          news_categories?: string;
          /** List of news topics separated by comma */
          news_event_types?: string;
          /** Page number */
          page?: number;
          /** Page size */
          page_size?: number;
          /** Published date in format (yyyyMMdd - 2023-04-01) to return news from */
          published_begin_date?: string;
        };
      };
      responses: {
        /** OK */
        200: {
          schema: definitions["NewsResponseDto"];
        };
        /** Unauthorized */
        401: unknown;
        /** Forbidden */
        403: unknown;
        /** Not Found */
        404: unknown;
      };
    };
    parameters: {};
  };
};

And the query:

// I had to change to `type: "module"` in package.json to get this working.
import createClient from "openapi-fetch"

createClient<
  paths
>({
  baseUrl: "https://api.salesintel.io",
  headers: {
    'Content-Type': 'application/json',
    'X-CB-ApiKey': (
      process
      .env
      .SALESINTEL_API_KEY!
    ),
  },
})
.get(
  "/service/news",
  {
    params: {
      query: {
        company_domains: (
          companyDomains
          .join(
            ','
          )
        ),
        page_size: 10,
      }
    }
  },
)
.then(({
  data, // `undefined`, but why?
  error, // `undefined`
}) => {
  if (
    error
  ) {
    throw error
  }

  if (
    !data
  ) {
    throw {
      message: "SalesIntel query has no data.",
      name: "NoSalesIntelDataError",
      status: 400,
    }
  }

  return data
})

How can this be reproduced / when did the error occur? Does the issue occur in a specific browser, or all browsers?

It occurs in TypeScript.

Expected result

I should be getting back the NewsResponseDto type for data.

Checklist

Sawtaytoes avatar Jul 05 '23 09:07 Sawtaytoes

Swagger v2 is missing MIME type (application/json) in the responses object that OpenAPI v3 requires. That’s where the type inference is failing here.

Though support for Swagger v2 isn’t in the roadmap at all for openapi-fetch, perhaps we could at least try to use schema if that exists directly below the status code. Beyond that, I think support could be difficult, but in simple cases like this, it may work as you mentioned.

drwpow avatar Jul 06 '23 22:07 drwpow

Swagger v2 is missing MIME type (application/json) in the responses object that OpenAPI v3 requires. That’s where the type inference is failing here.

Though support for Swagger v2 isn’t in the roadmap at all for openapi-fetch, perhaps we could at least try to use schema if that exists directly below the status code. Beyond that, I think support could be difficult, but in simple cases like this, it may work as you mentioned.

Strange you say Swagger v2 isn't supported. When I ran the generator in v6, it told me to use v5 to get Swagger v2 compatibility.

SalesIntel's API schema is available here. Is this what you're saying I could use?: https://api.salesintel.io/swagger/v2/api-docs

For my case, I don't have a choice on the OpenAPI version. I made my own API v3, but I don't control theirs.

I completely didn't understand what you said, but luckily, I had a v3 file to look at and see what you mean now. schema doesn't work because content is the property it's wanting.

OpenAPI v3:

200: {
  content: {
    "application/json": components["schemas"]["Account"]
  }
}

Versus Swagger v2:

200: {
  schema: definitions["NewsResponseDto"]
}

Since I'm writing the file myself, I could modify it to match v3's spec using string replacement:

import generateTypescriptFromOpenapiSchema from "typescript-openapi"

generateTypescriptFromOpenapiSchema(
  "https://api.salesintel.io/swagger/v2/api-docs",
)
.then((
  schema,
) => (
  writeFile(
    './src/schema.generated/salesintel.ts',
    schema, // I can regex this by-line where `schema: definitions` exists.
  )
))

Sawtaytoes avatar Jul 06 '23 22:07 Sawtaytoes

I did a manual copy-paste real quick and that does indeed fix the TypeScript issues:

200: {
  content: {
    "application/json": definitions["NewsResponseDto"];
  };
  schema: definitions["NewsResponseDto"];
};

image

Sawtaytoes avatar Jul 06 '23 22:07 Sawtaytoes

Here's my V2 -> V3 fix:

import generateTypescriptFromOpenapiSchema from "typescript-openapi"

const swaggerV2SchemaResponseRegex = /^(\s+)schema: (.+)$/

generateTypescriptFromOpenapiSchema(
  "https://api.salesintel.io/swagger/v2/api-docs",
)
.then((
  schema,
) => (
  schema
  .split(
    "\n"
  )
  .map((
    schemaLine
  ) => (
    schemaLine
    .includes(
      `          schema: definitions["`
    )
    ? (
      schemaLine
      .concat(
        "\n",
        (
          schemaLine
          .replace(
            swaggerV2SchemaResponseRegex,
            "$1content: { \"application/json\": $2 };",
          )
        ),
      )
    )
    : schemaLine
  ))
  .join(
    "\n"
  )
))
.then((
  schema,
) => (
  writeFile(
    "./src/schema.generated/salesintel.ts",
    schema,
  )
))

@drwpow Questions:

  1. Is this gonna break, or it'll be safe for this simple API?
  2. Is there a way the newest version can allow me to download from Swagger v2 APIs? It errors telling me to use v5. Unless patched, older versions can create security vulnerabilities, so I'd like to be on the latest. Not like the v3 API is changing anytime soon :p, so it should still be compatible.

Sawtaytoes avatar Jul 07 '23 07:07 Sawtaytoes

As you know, openapi-typescript’s latest version (v6+) only supports OpenAPI 3.0 and 3.1. v5 doesn’t have any known security vulnerabilities, and it will still receive security updates if any are found. The reason for dropping Swagger v2 support was mainly the additions in 3.1 that became too complicated to maintain in the same library as Swagger v2. While there are dozens of amazing contributors and maintainers that help, given the choice between buggy support for everything vs good support for 3.x, the latter seemed more useful (especially since the spec is 3+ years old now and most APIs have updated to 3.x no problem).

As for openapi-fetch, that only supports OpenAPI 3.0 and 3.1 for the same reasons—in order to add Swagger v2 support I’d have to fork it and maintain the Swagger v2 support on its own. Which we may do if there’s enough demand, but as it’s still in beta, I wanted to at least get the 3.x implementation further along before doing that.

To answer your questions:

  1. Is this gonna break, or it'll be safe for this simple API?

Yeah. I’d probably use a utility (like editor.swagger.io) to convert the Swagger v2 schema into OpenAPI 3.0 or 3.1 and store it somewhere locally. Most times the schema is simple enough it can do it perfectly, or do most of the work for you. Although you would have to maintain this yourself, the good news is that (hopefully) APIs change so infrequently that any update would only require a quick copy + paste. At any rate, it would be far less maintenance than trying to maintain a custom transformer like this.

  1. Is there a way the newest version can allow me to download from Swagger v2 APIs?

At the moment, no, but I’ll leave this ticket open if the need arises. It would have to its own import, e.g.:

import swaggerV2Client from 'openapi-fetch/swagger-v2';

I’m open to the idea of adding this in the future, if enough people need it. It would be much, much lighter effort than openapi-typescript, but would still require work and maintenance to keep it working and working well.

drwpow avatar Jul 07 '23 13:07 drwpow

Thanks for your response!

I think you nailed it on the head. I don't need Swagger v2 compatibility, I need to convert from Swagger v2 to OpenAPI v3. Once I've done that, everything just falls in place.

I'd prefer you guys added this as a separate npm library though.

Looking at the npm landscape, nothing's really supported: https://www.npmjs.com/search?q=swagger2openapi

The hack I have right now works, but being able to convert would be the ideal solution. Thanks for the recommendation!

Sawtaytoes avatar Jul 09 '23 07:07 Sawtaytoes

I've just stumbled on same issue, that some APIs are stuck with Swagger v2 and to create proper schemas for openapi-fetch I had to first parse it to Swagger v3 using swagger2openapi and than create schema using openapi-typescript

For me simple instruction how to do it in docs would be great. Like:

  1. Get json/yml schema for Swagger v2
  2. Parse it using swagger2openapi to OpenAPI v3
  3. Use openapi-typescript to generate types

Maybe in a solution would be to trigger swagger2openapi before generating types when Swagger v2 is detected (there is already a warning)

KajSzy avatar Jul 11 '23 11:07 KajSzy

Good call—definitely worth adding to the docs short-term. I don’t want to do anything automated, because swagger2openapi doesn’t work for every schema. But giving people a hint that they can manage this themselves is a good idea.

drwpow avatar Jul 11 '23 16:07 drwpow

Going to close this issue as Swagger v2 support isn’t in the maintenance budget for this project, especially since OpenAPI 4.0 is on the horizon (which this library will support) and most projects are now on OpenAPI 3.0.

drwpow avatar Jun 24 '24 15:06 drwpow