Closed drwpow closed 4 months ago
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:
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:
@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?
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:
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
Thanks to @joostme who pointed out that there's a workaround for my problem:
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 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 đ
Upgraded to v7 without issues. Generated file looks neat and tidy - especially when using external file refs. Thank you for this awesome job!
@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;
};
};
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.
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).
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?
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;
};
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 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)
@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.
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.
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).
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 default
s. 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.
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.
For sure! Please post back here as and when something changes
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 :
@ogjonixyz Thank you! Will track that as well. Appreciate all the testing and reporting issues while itâs still in beta.
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 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.
@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
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!
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?
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.
@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.
Also @sideroff to @WickyNilliamsâ point, duplicating operationIDs should probably throw an error because of how this library works. Will add that in the next preview release.
I've encountered two issues with v7.
openapi-typescript@next
and on main
. v6 works.The exception is coming from redocly whenever it calls its own function isMappingRef()
. I suppose I'll need to take this up with redocly, but posting here in case the issue is obvious and/or related to openapi-typescript
.
If I return document.parsed
early on L120 of redoc.ts I get types!
defaultNonNullable: false
, I have one unexpectedly different type. (there are other differences in the file but structurally the types appear to be equivalent)// v6
type V6 = {
ui_choice_labels: {
[key: string]: string;
} | null;
}
// v7
type V7 = {
ui_choice_labels: {
[key: string]: string | undefined; // union with undefined is unexpected
} | null;
}
The relevant part of the schema is:
"ui_choice_labels": {
"anyOf": [
{
"additionalProperties": {
"type": "string"
},
"type": "object"
},
{
"type": "null"
}
],
"default": null,
"title": "Ui Choice Labels"
}
I noticed this was unpinned as I checked back for updates. Should outstanding issues reported in this thread be moved to their own issues?
openapi-typescript@7.0.0-next.5 bug: require('openapi-typescript') cannot export the astToString method for example: var { astToString } = require('openapi-typescript');
openapi-typescript@7.0.0-next.5 bug: require('openapi-typescript') cannot export the astToString method for example: var { astToString } = require('openapi-typescript');
I'm running into the same issue, plus when running a script with ts-node, I get:
Could not find a declaration file for module 'openapi-typescript'. './node_modules/openapi-typescript/dist/index.cjs' implicitly has an 'any' type.
I noticed this was unpinned as I checked back for updates. Should outstanding issues reported in this thread be moved to their own issues?
Ah it was unpinned by mistake, but hopefully can resume getting the final tests in and releasing an RFC sometime soon.
That said, new issues are probably preferred, only for the reason to keep the notifications down for the people that commented here but werenât expecting to see bug reports đ
If you need a beautiful and easy-to-use openapi ui document, you can take a look at this openapi-ui
There are some great features here, specifically --enum.
I can't get this working with Redocly in @7.0.0-next.5 when using the resolve headers for basic auth
# used for API type generation
extends:
- minimal
resolve:
http:
headers:
- matches: https://raw.githubusercontent.com/myorg/**
name: Authorization
value: "Basic MYTOKEN"
I stepped through the Redocly code and there are multiple resolvers that make requests for the same file, kind of weird, one (or more) resolver instances isn't passed the config so it makes a unauthorized request and bombs the build.
I am not creating an issue since I don't know how to isolate this to openapi-typescript's use of Redocly or Redocly itself, just thought I would note somewhere since I'm not finding any search results.
Aside from this, I really don't want validation from Redoc forced in the same command where I generate types. I'm not sure why these operations are being combined.
@FirstWhack
Aside from this, I really don't want validation from Redoc forced in the same command where I generate types. I'm not sure why these operations are being combined.
FWIW, I couldn't get v7 to work when I last tried due to a problem with Redocly. If I manually commented out some code in redocly itself, everything worked and I got types that were identical to v6. Well, nearly identical - it's not clear if my change to redocly was the reason the types changed or not.
@FirstWhack
Aside from this, I really don't want validation from Redoc forced in the same command where I generate types. I'm not sure why these operations are being combined.
FWIW, I couldn't get v7 to work when I last tried due to a problem with Redocly. If I manually commented out some code in redocly itself, everything worked and I got types that were identical to v6. Well, nearly identical - it's not clear if my change to redocly was the reason the types changed or not.
Did you ever try just passing the contents as a string? I'm thinking of doing that. Redoc wants me to have a env var with an entire base64 encoded username/password prepared just to use Basic Auth. So I need to script before I call openapi-typescript to assemble such a thing from an actual token, inject as a env var. It's not very palatable even if it was working.
@FirstWhack
Did you ever try just passing the contents as a string? I'm thinking of doing that. Redoc wants me to have a env var with an entire base64 encoded username/password prepared just to use Basic Auth. So I need to script before I call openapi-typescript to assemble such a thing from an actual token, inject as a env var. It's not very palatable even if it was working.
I didn't, is this what you mean?
import fs from 'node:fs';
import openapiTS from 'openapi-typescript';
const schema = fs.readFileSync('openapi.json', 'utf8');
async function main() {
const types = await openapiTS(schema);
fs.writeFileSync('schema.ts', types);
}
main();
That gives me the same error as before from redocly: TypeError: mapping.startsWith is not a function
That gives me the same error as before from redocly:
TypeError: mapping.startsWith is not a function
Yep that's what I meant. For me the problem is just when retrieving the spec via URL... But if I don't retrieve it this way I don't think I can use any of the other redoc features? All I want from v7 is --enum, might look at a custom transformer for now.
Thank you for the great project @drwpow ! I am trying to migrate to the v7, I only noticed an issue with nullable enums.
When I run npx -y openapi-typescript@next test.json > test.ts
on this spec:
{
"openapi": "3.1.0",
"info": {
"title": "Minimal API with Nullable Enum",
"version": "1.0.0"
},
"paths": {
"/example": {
"get": {
"summary": "Get Example Value",
"responses": {
"200": {
"description": "Successful response",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {
"value": {
"type": "string",
"enum": ["option1", "option2", "option3"],
"nullable": true
}
}
}
}
}
}
}
}
}
}
}
I get:
For me value should be nullable: value?: "option1" | "option2" | "option3" | null;
Is it a bug ?
@gduliscouet-ubitransport yes thatâs a bug!
Testing the 7.0.0-next.8, with the --immutable --enum
CLI parameters, the enums are generated with a readonly modifiers:
export readonly enum OrderStatus { // TS1024: readonly modifier can only appear on a property declaration or index signature.
placed = "placed",
approved = "approved",
delivered = "delivered"
}
But enum can not have the readonly
modifier (as far as I know đ )
@JeanRemiDelteil great catch! Would love a PR to fix this (no need to open a separate issue)
I will see what I can do with the time I have at hand ! (Ok, let's discuss more in the PR if there is the need to.)
Another issue with v7 compared to v6 seems to be the handling of the --immutable flag for arrays.
with v7 I got:
readonly Pet: { readonly photoUrls: string[]; }
where in v6 it gives:
readonly Pet: { readonly photoUrls: readonly string[]; }
Is it by design ?
Another issue with v7 compared to v6 seems to be the handling of the --immutable flag for arrays.
with v7 I got:
readonly Pet: { readonly photoUrls: string[]; }
where in v6 it gives:
readonly Pet: { readonly photoUrls: readonly string[]; }
Is it by design ?
Nope! Not intentional. Fixing this bug now, and going through all the reported issues so far to work toward a stable release soon.
Upgraded to v7 on a pretty complex project with several openAPI definition involved without a single type error đ Thanks for the great job!
v7 Preview
openapi-typescript@7
is now in preview! đ The major additions to this library are:redocly.yaml
for API config (define your schemas there in one place, and useopenapi-typescript
to generate TypeScript without having to manage a separate config)--enum
flag for generating true TypeScript enums instead of unions (default behavior still unions)
### Features - ⨠**Feature**: automatically validate schemas with Redocly CLI ([docs](https://redocly.com/docs/cli/)). 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](https://redocly.com/docs/cli/configuration/) - ⨠**Feature**: bundle schemas with Redocly CLI - Any options passed into your [redocly.yaml config](https://redocly.com/docs/cli/configuration/) are respected - ⨠**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](https://openapi-ts.pages.dev/cli/) 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: ```diff + 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](https://astexplorer.net/)â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 - To migrate, youâll need to create a [redocly.yaml config](https://redocly.com/docs/cli/configuration/) that specifies your auth options [in the http setting](https://redocly.com/docs/cli/configuration/#resolve-non-public-or-non-remote-urls) - 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](https://openapi-ts.pages.dev/docs/cli/#multiple-schemas) for more info.Full Changelog
And dozens of small performance improvements and quality fixes.
Give it a try today with the
@next
flag: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.