Comments (14)
Since there are now quite a few requests for this, I’d love to add it. But there’s one problem: I think this would mean that the Node.js API for this library, which is currently synchronous, would have to be async. Because if it encounters remote schemas, it now has to fetch them (I know that if all $ref
s are local we could keep it sync, but I’d rather just allow for remote $ref
s and if it’s sometimes async, just make it always execute async).
This may cause a little disruption to some people using the Node.js API, but I think moving to async overall would be a win. CLI wouldn’t change.
What are peoples’ thoughts? 👍🏻 or 👎🏻 ?
from openapi-typescript.
Yeah I think that’s heading in the right direction. It‘ll be tricky to resolve, for sure, but possible. I think the right fix would be a lot of work, because it‘d involve resolving those files (and downloading them, as we currently support remote specs). So I want to split apart a short-term fix with a longer one.
Short-term
I think just setting this to any
for now would be a one-line fix to at least get this to generate. This would also add the logic we‘d need later to say “this is an external file.“ That external file will take work to resolve, but for now just set it to any
so we can get part of the spec, as opposed to none. Not perfect, obviously, but better.
Future Scope
Defining an interface only for the imported file would be tricky, because OpenAPI allows schema names that aren‘t TS-safe. And when you rename things, there can be collisions (README has a note on this). Plus, if that file contains a $ref
back to the original, that’d be tricky to resolve. Multiply if more files were added to the mix.
However, if you kept everything inline, that would work for TS, but what if test.yaml
had a reference back to the original spec? All it would take would be one circular reference to make this pattern unusable (and I think that‘d be almost certain in most cases).
I‘ll spend a bit more time thinking on it, but here are 2 valid options I can think of off the top of my head:
- Resolve it inline, but err on a circular reference. This would break down pretty quickly, as I think circular references are highly-encouraged. I think this would nix the main advantage of remote references like your issue points out.
- Invent a new interface just for external files. I think your suggestion would work with a minor tweak:
interface components {
{ // …
"$ref": ExternalFiles["test.yaml"]["foo"]
}
}
interface ExternalFiles {
"test.yaml": {
foo: // …
}
}
}
This would allow circular references back-and-forth, while being backwards-compatible with the current API.
I think the only downside would be if you were trying to generate types for both specs, you‘d have duplication. But I think that‘d be expected, right? Because both files reference each other.
WDYT about implementation?
from openapi-typescript.
FYI there are libraries out there implementing the RFC6901
an the $ref
draft. E.g.
https://www.npmjs.com/package/@apidevtools/json-schema-ref-parser
JSON Schema $Ref Parser is a full JSON Reference and JSON Pointer implementation that crawls even the most complex JSON Schemas and gives you simple, straightforward JavaScript objects.
It does resolve all references including external ones.
from openapi-typescript.
this would mean that the Node.js API for this library, which is currently synchronous, would have to be async
My opinion is that we should use the same approach to async schemas that ajv
has:
ajv.compile(schema: object): (data: any) => boolean | Promise < any >
So we do sync validation whenever we can. But if it requires extra work, we could KISS by always doing async. Also, readFileSync
is a hack and with modern SSD drives is a performance problem.
As for breaking changes, we can do it two ways:
- break it however we want, and increment the major version
- add
..Async
versions of the API
from openapi-typescript.
PR is up! Please take a look specifically at this test snapshot to get a feel for the newly-generated code: https://github.com/drwpow/openapi-typescript/pull/602/files#diff-884150d72ef4b660d8e83b0fcef66aeea1c8c1da9eddfebed53176ac43d4ebe7
from openapi-typescript.
This should be fixed at least. But when generating schemas, what would you like to see the generated types become for this?
from openapi-typescript.
Hey @drwpow, thats a good question. I'd say define a new interface? 🤔
interface test {
id: int
}
from openapi-typescript.
Hi @drwpow, thanks for your reply. I like the short term fix. This way the configuration would not throw.
On the interface with the refs I'm not so sure. This way when we use this inside our code we would have to know the filename inside the OpenAPI project to use the type inside the programs code. I don't really know how the OpenAPI spec defines what should happen with circular references. From what I understand the $ref-block is 'replaced' by the corrosponding code - that would indeed break down pretty fast.
It would be awesome to just be able to use that interface inside the code without having to use the components.$ref path.
Maybe namespaces could be handy here?
from openapi-typescript.
Oh you’re right. I made a mistake in my example. I should have illustrated that $ref
goes away. Agreed that you should be able to use the code without $ref
. I don’t think you should have to know the filename, either. Just internally, we do need a way to keep collisions apart, somehow, so we’d need some separation. But that may be more of an implementation detail.
I think the rest of it may become more clear as we try and prototype this out. One unknown for me is how much of the imported file you can access; I’d probably lean toward “only what the original spec references specifically.”
Anyways, I’ll get a quick fix out so the spec can generate, and will make a followup PR later for the longer-term fix (may take a while though).
from openapi-typescript.
v2.3.4
released with the quick workaround. Hopefully that unblocks you for now!
from openapi-typescript.
@nponeccop that‘s a great find, thank you! I was not aware of namespaced schemas like this. If this is a good solution, then we could definitely add support for remote schemas in a future release.
from openapi-typescript.
IMO it would be fine to make it async-only and publish a breaking change. I will note that the most common use case for this package is in some sort of build step, which doesn't typically involve asynchronous operations. That being said I can't imagine it creating too big a lift for someone to update a few build scripts to async to support this.
If we really need to keep backwards compatibility, we could alternatively provide two functions and log warn if a remote ref is encountered when using the sync version.
from openapi-typescript.
Just wanted to give an update on this: I’m making progress on this feature and it’ll likely go out in the next minor release!
Thanks to @nponeccop for the suggestion on json-schema-ref-parser. That was a big inspiration to unblocking this. I’ll have a PR up later this week that can act as a RFC for those interested.
from openapi-typescript.
Released this as openapi-typescript@next
. Going to do a little testing before releasing this as latest
.
Again, event though it’s a 4.0
breaking version, that will only be for Node.js API users which must now await
on a promise. CLI users should see no disruption and this should be backwards-compatible with generated schemas.
from openapi-typescript.
Related Issues (20)
- Schema with `default: null` becomes `default("null")` in generated Zod code HOT 2
- DELETE request fails with "Body cannot be empty when content-type is set to 'application/json'" HOT 1
- Error when compiling for CJS: "The current file is a CommonJS module..."
- Generated types causes compiler errors HOT 2
- [email protected] require('openapi-typescript') cannot export the astToString method HOT 3
- use openapiTS and astToString generate file type error in V7 HOT 1
- "Unexpected end of JSON input" with bun.js HOT 6
- Unauthorized errors when using openapi-fetch with SvelteKit HOT 3
- Filter invalid characters when transforming names from openapi to typescript
- typescript-fetch: dynamic fetch url
- FreeForm Object is generated as empty object HOT 1
- Feature clarification request: multiple schemas HOT 5
- Types missing for parameters with duplicate names but different locations HOT 1
- The 'styleguide' field is deprecated warning printed to console on every call to openapiTS() HOT 2
- Reduce the size of the package (v7) HOT 3
- Integration with tanstack-query HOT 2
- Change client headers HOT 1
- Header in createClient not considered when resolving request HOT 1
- Only generate models and export at root level HOT 1
- How to allow `DEPTH_ZERO_SELF_SIGNED_CERT`? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from openapi-typescript.