feat: export schemas types at root level

[jean-smaug]

Changes

Parse the schemas node in order to exports them as types at root level of the file.

How to Review

You can run examples with the --root-types flag 😄

It will add the following exports at root level (example with Stripe)

Checklist

• Unit tests updated
• docs/ updated (if necessary)
• pnpm run update:examples run (only applicable for openapi-typescript)

[changeset-bot]

⚠️ No Changeset found

Latest commit: dc9236d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[drwpow]

So this is a different approach to what I was expecting. I’m definitely against generating PascalCase INSTEAD of the core schema. But I like your idea of generating PascalCase types IN ADDITION to keeping the core schema intact. To be honest, no one’s suggested this before, but I like it!

However, this does need a few things before we ship it:

1. Adding to Node API edit: oops! You did this. I just missed it because it needs to be in the ctx object (see comment)

2. Tests. This needs tests to ship. I’d recommend first adding it to the options tests in index.test.ts. But we’ll also need additional tests to make sure it works on all example schemas.

3. Conflict resolution. Probably part of tests, but we’ll need to resolve when a schema has types foo-bar-baz and foo.bar.baz. Because this isn’t a conflict—it’s only a conflict in camelcase, and this library never has any bearing over how you write your schema. Whether that means we error with an exit code of 1 on conflict or just add a simple _2 suffix or something. But either way, users need to know when this generates conflicting types

4. Resolution to original types. I’d probably suggest this instead of rebuilding the entire type again:

   export type PaymentLinksResourcePhoneNumberCollection = components["schemas"]["payment_links_resource_phone_number_collection"];

   That would not only be more performant; it’d also reduce errors because then there’s not 2 independently-generated duplicates of the same type (which may have a bug in some obscure case that is hard to test for, and if the types are buggy, this library is useless!)

5. Handling of all components (not just components/schemas). I could be convinced if there’s a good argument to only include #/components/schemas, but I’d like to start with the default of “why not the entire components object?” Many schemas make use of all keys here (especially #/components/responses gets a ton of use). And the flag --root-types doesn’t suggest any sort of scoping to the schemas object alone—it suggests this is for your entire schema. Even if you predominantly use #/components/schemas doesn’t mean other people do, too.

   Perhaps even prefixing with Schema* or Parameter* or Response* would be good enough to separate them (or, to 3, possibly only doing that if there’s a conflict).

Again, still in favor of this approach overall! But it’s worth pointing out the original version of the library took the same approach but had to move away from this because it was unworkable for most schemas. The scoped namespacing handles conflict resolution, and generates all paths by its design—it mirrors schema authorship. So releasing a flag that promises friendly names does have to solve these hard problems, unfortunately. Otherwise we’ll just get a bunch of bug reports to fix them later, and it may be harder to do after it’s been integrated without rewriting it.

[drwpow]

nit:

Suggested change

	return output.join("\n").concat(schemasExportedTypes);
	// --root-types
	if (options.rootTypes) {
	const schemasExportedTypes = transformComponentsObjectToTypes((allSchemas["."].schema as OpenAPI3).components;
	output.push(schemasExportedTypes);
	}
	return output.join("\n");

This has the same execution as what you wrote, but it contains all the logic in one place rather than scattering it around in multiple places (IMO it’s weird for one option and one option alone to claim the last few lines of the final output; this change would make reordering at any point easier)

[jean-smaug]

I wish I could do this, but allSchemas is deleted line 112 of this file 😅

I imagine you did this on purpose, is there any counter-indication to remove this line ?

[drwpow]

Sorry; not sure I understand?

[drwpow]

let’s also add this to the ctx object so it’s more explicitly passed around

[jean-smaug]

To be honest, no one’s suggested this before, but I like it!

Thanks 😁

2. Tests. This needs tests to ship. I’d recommend first adding it to the options tests in index.test.ts. But we’ll also need additional tests to make sure it works on all example schemas.

For sure ! Before spending more times on it I wanted to ensure that I wasn't going against the philosophy of the project :)

3. Conflict resolution. Probably part of tests, but we’ll need to resolve when a schema has types foo-bar-baz and foo.bar.baz. Because this isn’t a conflict—it’s only a conflict in camelcase, and this library never has any bearing over how you write your schema. Whether that means we error with an exit code of 1 on conflict or just add a simple _2 suffix or something. But either way, users need to know when this generates conflicting types

Didn't knew about it, I'm not working a lot with OpenAPI, mostly consuming it.

4. Resolution to original types. I’d probably suggest this instead of rebuilding the entire type again:

👍 Agree would be more performant :)

5. Handling of all components (not just components/schemas). I could be convinced if there’s a good argument to only include #/components/schemas

Don't have any arg, since my initial need was fulfilled with the root schemas, I didn't dive deeper.

Thanks for feedback I'll work on it 😄

[drwpow]

Still in favor of this merging! But just marking as “changes requested” until it’s ready for review again.

[jean-smaug]

Still in favor of this merging! But just marking as “changes requested” until it’s ready for review again.

No problem 👍
I'm working on it today

[alex-aveva]

@jean-smaug @drwpow Is there any way to test it locally?
I installed that branch in my packages,json like this:
"openapi-typescript": "github:jean-smaug/openapi-typescript#export-types",
but when I try to call it with npx I'm getting some very odd version:
npm openapi-typescript --version

If I run it like this npx openapi-typescript ./test.json -o test.ds.ts --root-types it's reporting 6.5.3
✨ openapi-typescript 6.5.3

I can figure out from where 6.5.3 it is coming from.

Here's how I call it:
npx openapi-typescript ./test.json -o test.ds.ts --root-types
also tried
npx jean-smaug/openapi-typescript#export-types ./test.json -o test.ds.ts --root-types
it seems to try to install it but does not execute anything.

[drwpow]

@alex-aveva it’s because the original PR was based off of 6.5.3 so that’d be the reported version.

I’m not sure of the status of this PR; some tests were added but we still need a few more tests (such as testing conflicts are handled, testing generating rootTypes from external sources, and testing that shapes beyond #/components/schemas work as expected) in order to merge this.

I did rebase it against the 6.x branch since now main is tracking the upcoming v7 release.

[drwpow]

We’re not using this field in package.json. We’re tracking contributors via the docs site. Let’s remove it.

[alex-aveva]

@jean-smaug @drwpow I was able to test this today.
Setup steps (mostly for me so I can reproduce it next time):

• Clone repo to openapi-typescript

git clone https://github.com/jean-smaug/openapi-typescript.git 

• Install pnpm

curl -fsSL https://get.pnpm.io/install.sh | sh - 

• From openapi-typescript execute

npm run build:openapi-typescript

• Copy content of ~/openapi-typescript/packages/openapi-typescript to ~/.npm/_npx/<uuid>/node_modules/openapi-typescript, that's assuming that package was run prior with npx
  Perhaps there's easier way to force npx pulling binaries from ~/openapi-typescript/packages/openapi-typescript but I was not able to figure how to do that.

After that I was able trun npx openapi-typescript <schema> -o <client>.ds.ts --root-types, found following issues:

• Crashed on few schemas, see log

✨ openapi-typescript 6.7.0
file:///home/<username>/.npm/_npx/21d31b7563e6adbc/node_modules/openapi-typescript/dist/index.js:95
        for (const schema of Object.keys(typedComponents.schemas)) {
                                    ^

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at openapiTS (file:///home/<username>/.npm/_npx/21d31b7563e6adbc/node_modules/openapi-typescript/dist/index.js:95:37)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async generateSchema (file:///home/<username>/.npm/_npx/21d31b7563e6adbc/node_modules/openapi-typescript/bin/cli.js:83:18)
    at async file:///home/<username>/.npm/_npx/21d31b7563e6adbc/node_modules/openapi-typescript/bin/cli.js:192:7
    at async Promise.all (index 0)
    at async main (file:///home/<username>/.npm/_npx/21d31b7563e6adbc/node_modules/openapi-typescript/bin/cli.js:183:3)

• Types output was not what I expected, certainly did not look like that screenshot above, here's what I see

/**
 * This file was auto-generated by openapi-typescript.
 * Do not make direct changes to the file.
 */

export type AccessControlList = external['.']['components']['schemas']['AccessControlList'];

export type AccessControlEntry = external['.']['components']['schemas']['AccessControlEntry'];

export type Trustee = external['.']['components']['schemas']['Trustee'];

export type TrusteeType = external['.']['components']['schemas']['TrusteeType'];

Not exactly sure how these types can be used.

[drwpow]

Just checking in on this—again, still in favor of merging, but we still need:

1. Conflict resolution (with tests): not only test that conflicts are handled properly (components["schemas"]["foo.bar.baz"] vs components["schemas"]["foo-bar-baz"]), but that the resolution mechanism is well-defined and deterministic.
2. Producing schema objects beyond #/components/schemas. Also with the tests to assert this is being handled properly.
3. Tests, tests, tests. In case it wasn’t clear, this is a very complex feature add, so it will need many, many tests added even beyond the previous 2 points. Think of every way this can break with edge cases (e.g. how are $refs to external schemas handled? What about $refs of $refs of $refs? etc.).

Unfortunately, these are all requirements for merging, because this is a big feature. And big features place a burden on maintainers to carry longterm. This still has lots of sharp edges that need to be filed down to last in the project.

[alex-aveva]

@jean-smaug I'm happy to help with writing unit tests or anything really, just let me know.

[alex-aveva]

@drwpow @jean-smaug If there are no objections, I would love to finish this PR. I have some spare time to work on it. Please let me know!

[drwpow]

@alex-aveva I’m not sure about the state of this; at this point I think it’s fair to take over (and open a new PR)! The list of requirements still has to be met though 😅

[alex-aveva]

@drwpow I'll review the current state and aim to finalize it, thank you for the confirmation!

[knut-sverdrup-adsk]

If this PR is being picked up again, I'd like to chime in with my 2 cents on this part of the requested changes:

4. Resolution to original types. I’d probably suggest this instead of rebuilding the entire type again:
ts export type PaymentLinksResourcePhoneNumberCollection = components["schemas"]["payment_links_resource_phone_number_collection"];

That would not only be more performant; it’d also reduce errors because then there’s not 2 independently-generated duplicates of the same type (which may have a bug in some obscure case that is hard to test for, and if the types are buggy, this library is useless!)

Although aliasing like this would be more performant and arguably safe, from my point of view there are a couple of substantial downsides as well.

First off, aliasing does not retain docstrings, so any generated descriptions are lost. In my eyes, semantic descriptions are key enablers for ease-of-use, and with the current setup it is necessary to copy schema descriptions from generated file into the file which re-exposes types if you want to retain this information.

Secondly, it renders tools such as TypeDoc less valuable for documenting types, since the shape is not inferred from aliases.

The initial screenshot posted in the PR description shows output which is exactly what I want to achieve, and some benefits would be lost by the quoted suggestion.

@drwpow do these arguments make sense to you?

@alex-aveva if this PR ends up taking the approach of rebuilding types to give full access to the shape and docstrings directly, I'd be happy to contribute in finalising the PR -- don't hesitate to reach out :)

[tgross35]

First off, aliasing does not retain docstrings, so any generated descriptions are lost. In my eyes, semantic descriptions are key enablers for ease-of-use, and with the current setup it is necessary to copy schema descriptions from generated file into the file which re-exposes types if you want to retain this information.

It seems like maybe docstrings are preserved for the properties but just not the parent types? For example, I have the alias type LoginSuccess = components["schemas"]["LoginSuccess"] and hints still work for the fields:

But they do not seem to work for the parent types:

If this is consistent, maybe it would be OK to still export type aliases (i.e. not recreate the type with all fields, like I do in my example), but copy the schema's docstring to the type alias.

[fredrikssongustav]

@alex-aveva did you find time to pick this up? Would be a banger feature.

[alex-aveva]

@alex-aveva did you find time to pick this up? Would be a banger feature.
I eventually opted for ng-openapi-gen. The project I am working on uses Angular 17, and ng-openapi-gen seemed to be a better fit overall
cc @knut-sverdrup-adsk @drwpow

[drwpow]

First off, aliasing does not retain docstrings, … Secondly, it renders tools such as TypeDoc less valuable for documenting types … @drwpow do these arguments make sense to you?

Ah that’s a great callout. If it’s feasible, I’d like to still alias it but copy the docstring from the aliased type for this very reason. In 6.x this is difficult because there’s no resolver, but 7.x’s resolver makes lookups trivial and fast. So if this lands in 6.x, it could get this fix when being ported into 7.x.