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.

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

[Maintainer] Open API spec #65

Closed
kalinchernev opened this issue Mar 18, 2017 · 25 comments
Closed

[Maintainer] Open API spec #65

kalinchernev opened this issue Mar 18, 2017 · 25 comments
Assignees

Comments

@kalinchernev
Copy link
Contributor

kalinchernev commented Mar 18, 2017

I'll use the opportunity and the recent maintainers' activity to rise another high-level question I think will be good to start considering: the upcoming update in the swagger specification which becomes open api spec.

The situation we have at the moment as I see it is as following.

Community

High-level observations:

  • v3 rc was released recently, which probably means it's a good time to open the discussion
  • research efforts on tooling migration also has started by a prominent figure in the community
  • I think that with good strategy, swagger-jsdoc could target a good positioning in the documentation segment of the open api spec toolchain. Could also be in a general tooling of node.js, though I think that the momentum of the last year could be used as an advantage to seek a concrete positioning and purpose.

Although I'm not having the opportunity to be a full-time API architect or developer, the little time I manage to get from side projects and prototyping tasks, I already feel that swagger-jsdoc could be a useful tool when documentation in code can be reused as documentation out of code.

For example, here are 2 experiments I've been having recently:

As you can see, the workflows I'm experimenting with are going around the specification as core reusable component, and then trying to integrate swagger-jsdoc or other tool in "watch" mode to speed up the development of the spec together with the documentation generation and deployment.

Technical aspect

The 2 core dependencies this module has are doctrine and swagger-parser. So, I don't think we have to worry too much for these dependencies as probably we'll need to conditionally exchange only the swagger-parser or just use it as-is, depending on how it'll progress on the new specification.

Another question and aspect would be, would this plugin aim to handle 2 specifications at the same time? If yes, I think that i'd be convenient to change the way the module is used just a bit

var swaggerSpec = swaggerJSDoc(options);

to something like

// Get the module
const swaggerJsdoc = require('swagger-jsdoc');
// Create a parser depending on the specification and type of documentation blocks
const openApiDocsParser = swaggerJsdoc.createParser({version: 3, comments: jsdoc});
// Continue as now
const openApiSpec = openApiDocsParser(options);
// Or accept CPS
openApiDocsParser(options, spec => fs.writeFile('openapi.yaml', spec));

What I basically have in mind is that we can create a simple factory accepting some options before constructing the parser in order to be open for both new versions of the specification and different documentation styles if we would.

Your comments and feedback are welcome!


PS:

@chdanielmueller
Copy link
Member

Hi @kalinchernev,

Thank you for your well researched inputs.

I like this idea and I think with these changes it would be time for version 2.0.
Do you have time to take the lead on this and maybe reach out to some of the package's users for more help? I currently have a lot to do and can not help too much.

Thanks, Daniel

@drGrove
Copy link
Contributor

drGrove commented Mar 25, 2017

@kalinchernev,

This is great, we should create a "project" out of this and start breaking this down into tasks. I'll need to read a little further into Swagger v3, but more than glad to help.

Thanks,
Danny

@kalinchernev
Copy link
Contributor Author

kalinchernev commented Mar 26, 2017

Hi guys and thanks for the thumbs up!

Yes, I'd be interested into going further on this and take the lead. I'm also not fully knowledgeable of the progress of parsers and tools in the community, but want to make sure that when I do anything or research anything on this topic, I keep you in the loop.

Here's a short list of milestones I can propose as a start:

  • Create branch for the new version. I'd suggest to jump on v3 because it will be more straight-forward to reason when we speak about v3 of the swagger spec and will not take too much time for users to think about versions difference. Although this might seem a bit strange, I have to say we won't be the first to skip a version in making things easier for the users.
  • We can create issues to track progress on the sub-components: docblock parser, spec parser and general refactoring. Each can have a list of tasks in the issue description and can be used also as a thread to keep the others updated.
  • Update documentation - we already have meticulous documentation of the functions, so it would be really nice to re-use these to make the module even more accessible for contributors. Basically generating static pages as docstrap or similar out of the comment blocks and just placing them in docs/ folder which can be used by the github pages. If not this much, I'd just split the README into several more manageable and understandable chunks.

Most of all, are you ok with these ideas and the version bump?

@chdanielmueller
Copy link
Member

Yes, I am OK with your proposed changes.
Especially the documentation would benefit from being separated in smaller pieces.

@SomeoneRandom42
Copy link

Is there any updates on supporting OpenAPI 3.0? I am currently implementing swagger-jsdoc into our infrastructure and am looking forward to a few of the features of the new spec.

@kalinchernev
Copy link
Contributor Author

@SomeoneRandom42 with the existing parsing module, it's too early. Last time I took a look around this page there were some new projects on the market which could have support for v3.

@eljefedelrodeodeljefe
Copy link

Ref: still blocked by swagger-parser APIDevTools/swagger-parser#72

@kalinchernev
Copy link
Contributor Author

Because I started the issue, I feel obliged to ping back now https://www.npmjs.com/package/swagger-parser

@wibobm
Copy link

wibobm commented Jun 1, 2018

swagger-parser v5 supports OpenAPI 3.0

https://github.com/BigstickCarpet/swagger-parser/blob/master/CHANGELOG.md

@jmillan
Copy link

jmillan commented Jun 1, 2018

Hi,

First of all, congrats for this nice piece of code.

Just for the records. I wanted to test the openapi support for this library and made few minimal changes for a very basic support.

IMHO it would be very easy to port it to openapi if swagger support is dropped.

https://github.com/46labs/swagger-jsdoc/tree/openapi

@mrdj07
Copy link

mrdj07 commented Jun 26, 2018

Any news on this ?

@CarlosRosario
Copy link

CarlosRosario commented Jul 6, 2018

So now that swagger-parser v5 supports OpenAPI 3.0 does that mean that swagger-jsdoc will work with the OpenApi 3.0 specification ?

And if so, how do you toggle between Swagger 2.0 and OpenAPI 3.0 ?

@chdanielmueller
Copy link
Member

chdanielmueller commented Jul 10, 2018

@kalinchernev @drGrove @eljefedelrodeodeljefe
Hi guys,

Has any one of you time to look at the PRs regarding OpenAPI 3.0 support and the issues?
I sadly do not have time to look at this.

I think dropping the Swagger v2 Support would create a lot of issues with a lot of users.
If there is a possibility to support Swagger v2 and OpenAPI v3 we should go after it.
(Edit: Other opinions are welcome)

Thank you for your answer,
Daniel

@kalinchernev
Copy link
Contributor Author

I am generally interested into getting my hands dirty on this, however I'm not sure it'll be much appreciated to be honest.

On one side, previous ideas for getting the project into better shape in general were not followed up. On another hand, looking at the issue queue, it's obvious there are numerous issues with v2 which need to be treated before v3 and not to mention there are requests to even split the CLI to make a lighter version of the package #92 :)

For me, the main question is: what's the vision of swagger-jsdoc in general? Is it meant to be maintained on first place?

@chdanielmueller
Copy link
Member

@kalinchernev Thank you for your response.

The problem for me is that I no longer use swagger-jsdoc myself and do not have enough time to invest in it.

The thing I would like to avoid most is that this popular package loses it's value.
For me the purpose of this package has been to do one simple task and do it well. So maybe splitting the CLI from the rest of the package is a good idea?

I would be happy to "promote" you to the primary maintainer and set the vision yourself.
Would this be a solution for you?

@kibertoad
Copy link

I would like to contribute myself as well to keep the project alive, if there would be someone to review and merge :)

@mrdj07
Copy link

mrdj07 commented Jul 17, 2018

Hi ! I really needed it to be working with OpenAPI 3, so I forked @jmillan's branch and also added this pull request #115 and as of now I've had no issue generating my doc. Perhaps somebody wants to fork it back and complete it with tests ?

Here's my fork: https://github.com/mrdj07/swagger-jsdoc/tree/openapi

@kibertoad
Copy link

@chdanielmueller Is it possible to volunteer to maintain the project?

@kalinchernev
Copy link
Contributor Author

I looked at the fork of @jmillan and @mrdj07 and I think indeed changes for this swagger-jsdoc to support open api spec won't be too many for start.
@jmillan @mrdj07 can anyone open a pull request? Regardless of who does so, let's keep the communication open in the pull request and help each other.

Initial impressions from my side:

  • The only part which I see both of you have made same but I'd abstract a bit more is the if (!swaggerObject.swagger && !swaggerObject.openapi). A helper and/or a factory to initiate a parser depending on the version would not break backwards compatiblity
  • components integration seems ok on first sight, tests are needed
  • in a more general approach, I see that there are changes in the keys handled depending on the spec targeted. I would suggest considering that part to be refactored to be more reusable depending on the spec. For instance, together with the helper to decide upon a version, there could be one giving the list of properties allowed for a given spec

At any case, I'm positive about the suggestions I see - please do open a pull request for these and let's continue the discussion there!

@chdanielmueller
Copy link
Member

@kibertoad As you might have seen @kalinchernev has taken the lead in maintaining the project. Issues and Pull Requests are being worked on.

@kibertoad
Copy link

ah, great. Has @kalinchernev received write permissions for the repo?

@kalinchernev
Copy link
Contributor Author

Hi @kibertoad yes, I have permissions to approve and merge pull requests.

I will be happy to see people like you getting involved with enthusiasm. I was there a few years ago suggesting improvements while learning Node.js. And I guess because I know the project by improving it gradually @chdanielmueller gave me the trust to maintain it.

With time and maturity, many issues have been discovered. Suggesting fixes or helping maintainers understand what is the problem writing tests is super useful.

Unfortunately, with github is not easy to open the project for spontaneous contributions, i.e. if I want to suggest a small improvement in a pull request of someone else, adding a patch cannot be merged and contributed as far as I know. At the same time, there is a limitation on the number of seats available for contributors in the repository.

You can help in issues and pull requests and little by little get hold on the project. Then, naturally you will be in a position like me gaining trust and taking a contributor's seat in the project. And if needed, I can let mine even, if Daniel is ok with that as well.

My plan is to help on the major refactoring making the project more accessible #121 in terms of linting rules, etc. and the open api specifications. I believe putting the project back in shape for modern JS is a small but important step for the overall maintainability.

@eljefedelrodeodeljefe
Copy link

I will take a look. I'd scope this to OpenAPI 3 support from swagger-parser though.
Also, sorry for my inactivity.

@kalinchernev
Copy link
Contributor Author

Sure, the #122 is splitting helpers into files to offload thinking, but changes are in reality in 1 file which prepare properties differently depending on the spec version. Nothing new, only formatting changes which are mainly in #121

@kalinchernev
Copy link
Contributor Author

kalinchernev commented Aug 1, 2018

As I got some reviews and feedback on the pull request for this issue, I merged the pull request and released latest on npm. Including several tests added for the OpenAPI specification, and maintaining all existing tests for previous specification versions, I believe the release will be stable enough for shipping.

If any issues, feel free to follow-up in separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants