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

v7 Preview 🎉

Open drwpow opened this issue 2 years ago • 51 comments

v7 Preview

openapi-typescript@7 is now in preview! 🎉 The major additions to this library are:

  • ✨ Built-in validation using the Redoc CLI — no more needing an external tool
  • ✨ Respecting redocly.yaml for API config (define your schemas there in one place, and use openapi-typescript to generate TypeScript without having to manage a separate config)
  • ✨ Addition of the --enum flag for generating true TypeScript enums instead of unions (default behavior still unions)
  • 📘 A mini-rewrite to use the TypeScript AST instead of string mashing

Full Changelog

Features

  • ✨ Feature: automatically validate schemas with Redocly CLI (docs). No more need for external tools to report errors! 🎉

    • By default, it will only throw on actual schema errors (uses Redocly’s default settings)
    • For stricter linting or custom rules, you can create a redocly.yaml config
  • ✨ Feature: bundle schemas with Redocly CLI

  • ✨ Feature: add enum option to export top-level enums from schemas (#941)

  • ✨ Feature: add formatOptions to allow formatting TS output

  • ✨ Feature: header responses add [key: string]: unknown index type to allow for additional untyped headers

  • ✨ Feature: allow configuration of schemas via apis key in redocly.config.yaml. See docs for more info.

Breaking Changes

  • ⚠️ Breaking: The Node.js API now returns the TypeScript AST for the main method as well as transform() and postTransform(). To migrate, you’ll have to use the typescript compiler API:

    + import ts from "typescript";
    
    + const DATE = ts.factory.createIdentifier("Date");
    + const NULL = ts.factory.createLiteralTypeNode(ts.factory.createNull());
    
      const ast = await openapiTS(mySchema, {
        transform(schemaObject, metadata) {
        if (schemaObject.format === "date-time") {
    -       return schemaObject.nullable ? "Date | null" : "Date";
    +       return schemaObject.nullable
    +         ? ts.factory.createUnionTypeNode([DATE, NULL])
    +         : DATE;
          }
        },
      };
    

    Though it’s more verbose, it’s also more powerful, as now you have access to additional properties of the generated code you didn’t before (such as injecting comments).

    For example syntax, search this codebae to see how the TypeScript AST is used.

    Also see AST Explorer’s typescript parser to inspect how TypeScript is interpreted as an AST.

  • ⚠️ Breaking: Changing of several CLI flags and Node.js API options

    • The --auth, --httpHeaders, --httpMethod, and fetch (Node.js-only) options were all removed from the CLI and Node.js API
    • You can also set your fetch client in redocly.yaml as well.
    • --immutable-types has been renamed to --immutable
    • --support-array-length has been renamed to --array-length
  • ⚠️ Breaking: Most optional objects are now always present in types, just typed as :never. This includes keys of the Components Object as well as HTTP methods.

  • ⚠️ Breaking: No more external export in schemas anymore. Everything gets flattened into the components object instead (if referencing a schema object from a remote partial, note it may have had a minor name change to avoid conflict).

  • ⚠️ Breaking defaultNonNullable option now defaults to true. You’ll now need to manually set false to return to old behavior.

  • ⚠️ Breaking: Remove globbing schemas in favor of redocly.yaml config. Specify multiple schemas with outputs in there instead. See Multiple schemas for more info.

And dozens of small performance improvements and quality fixes.

Give it a try today with the @next flag:

npm i openapi-typescript@next

Testers wanted 🙏

Will pin this issue for a while while people kick the tires and report bugs while a release candidate is developed. For many people, it should be a drop-in replacement; for others, it will only require a few minor changes.

If people could report any errors/issues they face while trying this, that’d be much appreciated. Any issues posted here or as separate issues will help patch holes in the docs site.

drwpow avatar Oct 06 '23 00:10 drwpow

Great to hear that openapi-typescript will support external refs! Thanks a lot for that :)

I did quick test with our project and ran into a typing issue.

When extending a model using allOf...

    PurchasableProduct:
      type: object
      properties:
        price:
          $ref: '#/components/schemas/Price'
      required:
        - price
      allOf:
        - $ref: '#/components/schemas/Product'

... the generated code does not compile:

image

Our backend colleagues use the same OpenAPI file using the openapi-generator kotlin-spring plugin, so it seems to a be valid spec.

Here's an example repo to reproduce the issue:

https://github.com/luchsamapparat/openapi-typescript-7-bug

luchsamapparat avatar Oct 06 '23 06:10 luchsamapparat

@luchsamapparat the generated code seems to be working correctly; the error seems to be coming from the fact your schema specifies price as a required property but no such property exists. Does removing that fix the error?

drwpow avatar Oct 06 '23 13:10 drwpow

If you have another look at my example, the price property is not only defined as required, but also as a property directly below the properties: line.

You can copy and paste the full example OpenAPI YAML from my repo into the Swagger Editor which parses it correctly:

image

luchsamapparat avatar Oct 06 '23 22:10 luchsamapparat

Would be nice to support x-enum-varnames property.

UPD: Well, I think I did it. Check the PR https://github.com/drwpow/openapi-typescript/pull/1374

ElForastero avatar Oct 07 '23 17:10 ElForastero

Thanks to @joostme who pointed out that there's a workaround for my problem:

image

https://github.com/luchsamapparat/openapi-typescript-7-bug/commit/638870cbd25bcb6bfc775b781bfc18721ba52c89

luchsamapparat avatar Oct 09 '23 08:10 luchsamapparat

running with --immutable flag seems to generate invalid ts: export readonly interface paths {}

see:

npx openapi-typescript@next https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/examples/v3.0/petstore.yaml --immutable | head

error:

'readonly' modifier can only appear on a property declaration or index signature.ts(1024)

stefanprobst avatar Oct 14 '23 06:10 stefanprobst

@stefanprobst good catch 🙏! Will add some immutable tests to make sure the types are valid.

Sidenote: I did learn through the v7 rewrite that TypeScript’s AST does let you generate invalid TS without stopping you. Which is fun 🙃

drwpow avatar Oct 19 '23 15:10 drwpow

Upgraded to v7 without issues. Generated file looks neat and tidy - especially when using external file refs. Thank you for this awesome job!

maslennikov avatar Oct 25 '23 14:10 maslennikov

@drwpow may it be the case that there were some changes in handling of the | undefined portion of additionalProperties typings? I think, I am noticing now something of it.

Related issues: #1018, #1267

Input:

    CityLinks:
      type: object
      required: [socials]
      properties:
        socials:
          type: object
          additionalProperties:
            type: string

Output

         CityLinks: {
            socials: {
                // There was no `| undefined` in later v6 versions
                [key: string]: string | undefined;
            };
        };

maslennikov avatar Oct 25 '23 15:10 maslennikov

Why is it that we have a ton of never now in v7 whereas they did not exist in v6? it makes the generated types a fair bit more verbose to read.

CleanShot 2023-11-02 at 22 08 55@2x

tonyxiao avatar Nov 03 '23 05:11 tonyxiao

Why is it that we have a ton of never now in v7 whereas they did not exist in v6?

Great question! That actually has to do with how TS handles discriminated unions within other tools like openapi-fetch. For example, In v6, comparing the response objects between 2 or more endpoints would yield a very complex union because each would have different combinations of request methods—often each would be a completely different permutation of keys. And to TS, comparing objects with different keys are “apples to oranges” even if we don’t intend for them to be.

But when we add never in v7 and TS sees every request object has the same keys, it now knows it’s comparing “apples to apples” and can make better inference decisions (example). And as a result, writing type utilities becomes easier and safer to do (some edge cases were fixed in openapi-fetch by this change).

So it not only helps TS do its job better, and makes more advanced utilities built off openapi-typescript better, but I think the never additions are a more “correct” type in general—they make it clear where there could be an addition to your schema according to the spec, but it has been decidedly opted out of (or is not used yet).

drwpow avatar Nov 03 '23 10:11 drwpow

Gotcha, this is super helpful to know, thanks for the link to the reference article too! does this result also in a speed up in the auto complete type hints as a result?

tonyxiao avatar Nov 03 '23 20:11 tonyxiao

hey i think i have found an issue with v7. i've dug into it and i have found the source of the issue...

a number of my endpoints can return a 204 status, which has no content. for context, my API is following the JSON:API standard: https://jsonapi.org/format/#crud-creating-responses-204.

so the generated types look like this:

responses: {
  201: {
    headers: {
      [name: string]: unknown;
    };
    content: {
      // ...
    };
  };
  204: {
    headers: {
      [name: string]: unknown;
    };
    content?: never; // <-- this is the source of the issue
  };
  // ...
};

the potentially undefined content for 204 trips up the SuccessResponse type utility, causing it to resolve to never.

Here's a typescript playground which contains an abridged version of my generated schema and some code at the bottom to demonstrate the issue. if you remove the ? from the 204 content you will see that the you get the correct type. I am guessing a NonNullable somewhere in the type utilities will fix this

I do not have this issue with v6.7.0

Edit: for completeness, the 204 response looks like this when generated with 6.7.0. here it is not potentially undefined:

204: {
  content: never;
};

WickyNilliams avatar Nov 06 '23 11:11 WickyNilliams

Another issue sorry!

In v7, if a request body parameter has a default value, it is treated as required/non-optional in the generated types.

requestBody:
  content:
    application/vnd.api+json:
      schema:
        required:
          - data
        properties:
          data:
            type: object
            required:
              - type
            additionalProperties: false
            properties:
              type:
                type: string
              id:
                type: string
              attributes:
                type: object
                properties:
                  first_name:
                    type: string
                    maxLength: 100
                  last_name:
                    type: string
                    maxLength: 100
                  email:
                    type: string
                    format: email
                    maxLength: 100
                  language:
                    type: string
                    nullable: true
                    default: en
                    maxLength: 20
                required:
                  - email
                  - initials

this will generate a type like:

requestBody?: {
  content: {
      "application/vnd.api+json": {
          data: {
              type: string;
              id?: string;
              attributes?: {
                  first_name?: string;
                  last_name?: string;
                  /** Format: email */
                  email: string;
                  /** @default en */
                  language: string | null; // <-- ❌ not optional?
              };
          };
      };
  };
};

i believe this is incorrect, given the openapi spec on default (emphasis mine):

Use the default keyword in the parameter schema to specify the default value for an optional parameter. The default value is the one that the server uses if the client does not supply the parameter value in the request

in v6, the generated type is as expected:

requestBody?: {
  content: {
    "application/vnd.api+json": {
      data: {
        type: string;
        id?: string;
        attributes?: {
          first_name?: string;
          last_name?: string;
          /** Format: email */
          email: string;
          /** @default en */
          language?: string | null;
        };
      };
    };
  };
};

here's a stackblitz demonstrating the issue. it will output to schema.d.ts when it starts up https://stackblitz.com/edit/stackblitz-starters-dh6zug

WickyNilliams avatar Nov 06 '23 13:11 WickyNilliams

@WickyNilliams thanks for flagging! I will investigate that specifically. There are other breaking changes I’m tracking that will require a breaking version change for openapi-fetch (but fortunately the breaking change is less code since 7.x makes inference simpler/safer). Right now it’s known openapi-fetch only works with 6.x, and an upcoming breaking version of openapi-fetch will be released for 7.x (as a fast follow)

drwpow avatar Nov 06 '23 15:11 drwpow

@WickyNilliams also for the default issue, that seems like a bug. 7.x enables --default-non-nullable by default so it’s intended to have that behavior. But is just missing some relevant tests it seems.

drwpow avatar Nov 06 '23 15:11 drwpow

Right now it’s known openapi-fetch only works with 6.x,

is this mentioned in the docs? i looked in multiple before trying out v7, and since i didn't spot anything i assumed that they were compatible. might be worth sticking a big banner somewhere :D or perhaps list openapi-typescript as a peer dependency with a v6 range? but i guess peer deps can be their own can of worms

i have pinned to v6 in the meanwhile. glad to hear v7 enables some simplification of the fetcher!

7.x enables --default-non-nullable by default so it’s intended to have that behavior. But is just missing some relevant tests it seems.

ah i see that in the full changelog above now. the docs site itself still says it defaults to false by the way. is there some explanation somewhere for that change? in my (lacking-in-context and likely uninformed) opinion anything that diverges from what the schema explicitly defines should be opt-in. so i'd be curious on the background.

WickyNilliams avatar Nov 06 '23 15:11 WickyNilliams

might be worth sticking a big banner somewhere :D

A note on the docs would be a good idea! Will add that.

is there some explanation somewhere for that change?

Mostly from this issue, and if you search you’ll find other instances where it seemed like more users were expecting this behavior than not. And it seems like the spec suggests this should be the default behavior? Open to feedback if I’m misunderstanding something; I had just flagged this as a deviation from the specification (and with a spec deviation + multiple issues raised about the default behavior seems like a good hint to change it).

drwpow avatar Nov 06 '23 15:11 drwpow

in my reading i think there is a sutble difference between inputs and outputs. for outputs/responses, the set of non-optional fields should be the union of required and those with defaults. for inputs/requests, it is only those which are required.

reading the linked issue, i think that's what they're asking also. see the second bullet point (emphasis theirs):

But, when the consumer is writing the value can be provided or not so that the generated type should also be optional instead of required

i found this related issue https://github.com/drwpow/openapi-typescript/issues/584 which i think is talking about default values in responses being required.

there's also this comment which says:

So if a default value is set on a request parameter, it cannot be interpreted as a required parameter.

WickyNilliams avatar Nov 06 '23 16:11 WickyNilliams

Ah thank you that is a really great distinction. Will also add that to the test suite. That may mean some nuances of --default-non-nullable may change (maybe; maybe not). But breaking versions are the opportunities to make these improvements and dig deeper on how everything works.

drwpow avatar Nov 06 '23 16:11 drwpow

For sure! Please post back here as and when something changes

WickyNilliams avatar Nov 06 '23 17:11 WickyNilliams

I need to to create types from multiple remote sources but I keep getting TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be "localhost" or empty on linux error. Tried with local files and still the same issue. This is the redocly.yaml : image

ogjonixyz avatar Nov 08 '23 10:11 ogjonixyz

@ogjonixyz Thank you! Will track that as well. Appreciate all the testing and reporting issues while it’s still in beta.

drwpow avatar Nov 08 '23 14:11 drwpow

Hello. Got some issue. In yaml we have refs with schema field. And lib try to parse schema as a valid key to get data, but should ignore it in response

responses:
    '200':
      description: Info about foo
      content:
        application/json:
          schema:
            $ref: '../../schemas/foo-api/foo
responses: {
                200: {
                    headers: {
                        [name: string]: unknown;
                    };
                    content: {
                        "application/json": {
                            content?: paths["/foo/{fooId}"]["get"]["responses"]["200"]["content"]["application/json"]["schema"][];
                        } & {
                            count?: number;
                        };
                    };
                };
}

omgpiu avatar Nov 08 '23 14:11 omgpiu

@omgpiu could you make a separate issue please? Ideally with your full schema, version of openapi-typescript, and the full generated response. Just from the info provided I’m not sure what’s going on.

drwpow avatar Nov 18 '23 22:11 drwpow

@ogjonixyz I can’t say for sure, but it looks like in your redocly.yaml config, you’re not matching the correct letter case for the file? In many Linux filesystems, they’re case-sensitive. And that error message usually happens when you’re pointing to a nonexistent file

drwpow avatar Nov 18 '23 22:11 drwpow

Released a new version of @next with a few bugfixes. Still going to hunt down some remaining bugs and add a few more tests before releasing a RC. But it’s looking close!

drwpow avatar Nov 19 '23 00:11 drwpow

Hello! I noticed that with v7 you can end up with an invalid ts file if you have multiple paths that use the same operationId. For example using the following schema:

{
  "openapi": "3.0.0",
  "paths": {
    "/v1/path": {
      "post": {
        "operationId": "Operation1",
        "parameters": [],
        "responses": { "200": { "description": "" } },
        "tags": ["tag"]
      }
    },
    "/v2/path": {
      "post": {
        "operationId": "Operation1",
        "parameters": [],
        "responses": { "200": { "description": "" } },
        "tags": ["tag"]
      }
    }
  },
  "components": {
    "schemas": {}
  }
}

you get an additional duplicated Operation1 prop in the operations interface. I managed to remove the duplicates by filtering them out from the ts.Node[] that the default export function returns. Are there any plans on changing this behaviour?

sideroff avatar Nov 21 '23 16:11 sideroff

FYI the swagger docs have this to say

operationId is an optional unique string used to identify an operation. If provided, these IDs must be unique among all operations described in your API.

https://swagger.io/docs/specification/paths-and-operations/

WickyNilliams avatar Nov 21 '23 17:11 WickyNilliams

@ogjonixyz I can’t say for sure, but it looks like in your redocly.yaml config, you’re not matching the correct letter case for the file? In many Linux filesystems, they’re case-sensitive. And that error message usually happens when you’re pointing to a nonexistent file

Ah sorry—I encountered the bug. That bug is when you don’t explicitly pass in a --redoc flag and it tries to locate one (and when it does, it only returns a partial path and not a full one, hence the URL error), and we weren’t testing for that. Adding a test for that and pushing a fix.

drwpow avatar Nov 22 '23 03:11 drwpow