Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add external ref support #414

Closed
wants to merge 1 commit into from
Closed

add external ref support #414

wants to merge 1 commit into from

Conversation

dradetsky
Copy link

Unless I missed something, dtsgen does not currently support refs of the form

$ref: './path/to/doc.yaml#/path/to/SomeObject

I recently made a pr to add this to another library (oazapfts), so as I'm having some trouble with that library, I thought I would do a quick PR which does basically the same thing I did for that library.

I'm not necessarily suggesting this PR should be accepted as-is (although that's up to you). This is just enough for you to confirm it works, so we can discuss what would actually make the PR acceptable.

I think this closes #321 . I'm not totally sure I understand all that guy's requirements w/r/t other directories, but this does resolve a ref like $ref: './other-dir/extern.yaml#/PartnerInfo'.

@horiuchi
Copy link
Owner

@dradetsky Thank you for your comment.
Reference resolution (such as $ref) is complicated, so I'm willing to replace the implementation with a library. If you know of an external library that is better than the current implementation, it's a good idea to use it.
I'd like to do something about it, especially since the support for OpenAPI is weak at present.

@horiuchi horiuchi closed this May 15, 2020
@horiuchi horiuchi reopened this May 15, 2020
@dradetsky
Copy link
Author

@horiuchi

I don't know much about this specific library except:

  1. It's fairly widely used. I've seen it all over, such as in openapi-backend, which I'm using.

  2. The docs are fairly good, which is why I was able to find out so easily that this would be a solution. See e.g. here.

@silesky
Copy link
Contributor

silesky commented May 29, 2020 •
edited
Loading

@horiuchi json-schema-to-typescript uses https://www.npmjs.com/package/json-schema-ref-parser, which is by far the most popular ref parser and supports sub references and external refs. Worth taking a look?

@exoego exoego mentioned this pull request May 30, 2020
Closed
silesky
silesky reviewed May 31, 2020
View reviewed changes
src/cli.ts
});
});
return Promise.all(files.map(async (file: string) => {
return await SwaggerParser.bundle(file)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don’t need/want this “await”?

@horiuchi
Copy link
Owner

@silesky Thank you for your information!
I tried to implement json-schema-ref-parser briefly before, but the type didn't fit, so I gave up on it then.
In the new version, it's possible to add a plugin for preprocessing, so you might be able to use it there.

@horiuchi horiuchi mentioned this pull request Jul 29, 2020
Open
@horiuchi
Copy link
Owner

Thank you for raising the discussion.
I'm closing this request for now. I've created an Issue instead, so we'll continue there.

@horiuchi horiuchi closed this Jul 29, 2020
@JBBianchi JBBianchi mentioned this pull request Apr 29, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silesky silesky silesky left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for relative references
3 participants
@dradetsky @horiuchi @silesky