-
Notifications
You must be signed in to change notification settings - Fork 228
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
Add support for jsDoc filtering #111
base: master
Are you sure you want to change the base?
Conversation
Hi @identityclash I can't quite understand the scenario this change works on. |
Hi @kalinchernev , sorry for the late reply. I'll get back to you on creating an example for this, as I believe that this could be quite useful especially for a project using Concisely, as an example, say you have 5 APIs for a certain server, using the filtering mechanism in this PR would allow you to show only the first two API documents for client X, and the last three API documents for client Y. Another way would be to allow an mix and match of the API documents to be shown to a particular client. |
Hello @kalinchernev, I appended the manner of usage in the The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @identityclash and thanks for the suggestion!
In general, I like your idea about adding a feature to apply operations on annotated documentation.
First thought reading your expanded explanation - we need to add "hooks". This would be the more scalable way of adding functions making different types of changes. For example, at the moment you are trying to remove parts of the documentation (reduce), while others may want to enrich parts of the documentation, or other operations before the parsing, after the parsing, etc.
In relation to this, I think the first step would be to discuss what would be the most accessible way to "flag" comments you would like to modify. For instance, if it were me, I'd approach the problem by adding either a new type of annotation like @swagger
to flag I want to modify this part of the comment, or rely on tagging which is native to the specification and use it in your reduce function attached on the definition object.
These are my thoughts on first read. Please let me know if I've managed to grasp your ideas about the feature?
Thanks!
@@ -13,6 +13,9 @@ const options = { | |||
}, | |||
}, | |||
apis: ['./routes.js'], // Path to the API docs | |||
jsDocFilter: (jsDocComment) => { // Optional filtering mechanism applied on each API doc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, it feels a bit weird to return a boolean after receiving an argument. I mean, if this is meant to be a flag for a logic to be attached, I'd personally prefer something like jsDocFilter: true
and pass the function on another property of the definition object. Or maybe simply attach the function which returns a reduced object.
@@ -34,6 +37,10 @@ app.get('/api-docs.json', function(req, res) { | |||
}); | |||
``` | |||
|
|||
- `options.jsDocFilter` is a function which accepts only one variable `jsDocComment`. This `jsDocComment` represents each route documentation being iterated upon. | |||
|
|||
If you want to optionally perform filters on each route documentation, return boolean `true` or `false` accordingly on certain logical conditions. This is useful for conditionally displaying certain route documentation based on different server deployments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the comment above, I'd personally prefer to keep boolean and object types separate in terms of logic. For instance, jsDocFilter
can be simply a reducer function. If the function is set, it's obviously true
:)
// This function must return boolean. `true` to display, `false` to hide. | ||
const docDescription = jsDocComment.description; | ||
|
||
const features = docDescription.indexOf('feature') > -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the approach of storing checks in variables. For readability, you can name them hasFeatureX
, hasFeatureY
, etc. Also, since we are having modern JS version, you can freely use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes
} | ||
|
||
// featured route documentation | ||
if (features) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the place where I'd change the logic to return a reduced version of the docs.
Note that the filter only reads keywords above the `@swagger` identifier. | ||
```javascript | ||
/** | ||
* featureX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried using tags for grouping endpoints per given logical separation? https://swagger.io/docs/specification/grouping-operations-with-tags/
I need this too, we have a public and a private api. Would be nice if using tags we could create 2 or more specs.json's |
@etiennea it's been quite some time since the last time I put thought on this subject. Glad you bump it, though I can't promise a fast solution, as I don't quite understand the request even after reading through my old comments in the communication above. As a quick suggestion: you can maybe use https://www.npmjs.com/package/patch-package and modify this function https://github.com/Surnet/swagger-jsdoc/blob/master/src/utils.js#L36 so that you use the swagger-jsdoc package, but have a single place to do if/else logic based on your custom annotations such as @myapi1, @myapi2, etc. If you also have in mind a way to solve the issue: please go ahead. Current master is for 6.x CommonJS, and 7.x is in RC and can accept breaking changes, more or less the same as 6.x with more tests and ESM. Whichever is more convenient to express suggestions, they are welcome |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This PR allows support for filtering JSDocs through the use of the optional
jsDocFilter
function in order to selectively output certain documentation of choice.