-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
docs: openapi v3 decorators #1145
Conversation
docs/site/Decorators.md
Outdated
@@ -132,56 +132,181 @@ accordingly or do a composition of them: | |||
For more usage, refer to [Routing to Controllers](controllers.htm#routing-to-controllers) | |||
|
|||
### Parameter Decorator | |||
|
|||
Syntax: see [API document](https://github.com/strongloop/loopback-next/blob/master/packages/openapi-v3/src/parameter-decorator.ts#L18-L29) |
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.
Did you perhaps mean API documentation
?
Also, could you please point to http://apidocs.loopback.io instead of TypeScript source?
If we decide to keep the link to a ts file, then please make sure the link points to a specific revision, not the master
branch. Otherwise the link will point to wrong line numbers soon, as the code will evolve.
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.
@bajtos oops a typo, there is a bug of ts
doc parsing function:
Given a function and namespace with the same name, it only parses the namespace, but could not detect the function. That's why I point it to the code as a workaround.
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.
Interesting...seems people have different opinions on the link's branch. I use "a specific version" before but Kevin corrected me to use master....personally I agree with "a specific version".
Appreciate more opinion from team @strongloop/lb-next-dev , for the code links in documentation, would you point them to master
branch or a specific version?
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.
It is often that the line numbers change over time, so I'd avoid pointing to the source code, no matter which branch it is pointing to. My question to you, @jannyHou, would be.. what do we want to show to the users that we cannot show in a more static document, like API docs?
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.
@dhmlau My link points to the tsdoc of source code :( I agree that this is the last choice...while could not think of a better idea as workaround
docs/site/Decorators.md
Outdated
refer to [TypeScript Decorator Documentation](https://www.typescriptlang.org/docs/handbook/decorators.html) | ||
You can find the specific usage in [Writing Controller methods](controller.htm#writing-controller-methods) | ||
|
||
*We are supporting parameter location 'cookie' in the feature, track the feature in story* |
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.
The parameter location cookie
is not supported yet, see #997.
docs/site/Decorators.md
Outdated
``` | ||
|
||
or | ||
Writing the whole parameter specification is tedious, we create shortcuts to define params with pattern `@param.${in}.${type}(${name})`: |
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.
... tedious, so we've created shortcuts to define the parameter with the pattern ...
docs/site/Decorators.md
Outdated
or | ||
Writing the whole parameter specification is tedious, we create shortcuts to define params with pattern `@param.${in}.${type}(${name})`: | ||
|
||
- in: the parameter location, one of the following values: `query`, `header`, `path`. |
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.
... location. It can be one of the ...
docs/site/Decorators.md
Outdated
Writing the whole parameter specification is tedious, we create shortcuts to define params with pattern `@param.${in}.${type}(${name})`: | ||
|
||
- in: the parameter location, one of the following values: `query`, `header`, `path`. | ||
- type: a [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types), |
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.
The wording for this bullet point is a bit awkward. Could you rewrite it so that it's clearer what type
is for?
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.
@shimks could you elaborate more about which part confuses you?
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 moved take specifying a data get from query....
to next paragraph, see if it improves :)
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.
should it be the common name
instead of a common name
?
Also, is type
the type of the parameter? If so, not sure if we need the type specifying a data..
.
docs/site/Decorators.md
Outdated
- type: a [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types), | ||
take specifying a data get from query as an example, a list of available shortcuts can | ||
be found in [API document](http://apidocs.loopback.io/@loopback%2fopenapi-v3/#param.query). | ||
- name: name of the parameter, should be a `string`, |
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.
... parameter. It should be a 'string'
docs/site/Decorators.md
Outdated
|
||
An equivalent example using shortcut decorator would be: |
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.
... using the shortcut decorator ...
docs/site/Decorators.md
Outdated
*About decorating models and the corresponding OpenAPI schema, please check* | ||
*[model decorators](#model-decorators)* | ||
|
||
Then a typical usage of `@requestBody` is specifying a parameter's type as the model class, |
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.
A typical usage of '@requestBody' is specifying a parameter as a requestBody by decorating it
or something along that line.
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.
@shimks I still prefer to keep "then" :)
There is some code snippet in between, but the context is:
When the decorated argument is a custom type or a model, we recommend you define a model class with
@model
and@property
. Then a typical usage of@requestBody
is specifying a parameter's type as the model class, and applying the decorator on it.
docs/site/Decorators.md
Outdated
} | ||
``` | ||
|
||
For the simplest use case, you can leave the input of `@requestBody` empty, we automatically |
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.
... empty since we automatically ...
docs/site/Decorators.md
Outdated
``` | ||
|
||
For the simplest use case, you can leave the input of `@requestBody` empty, we automatically | ||
detect the type of `user`, generate the corresponding schema for it. |
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.
... type of 'users' and generate the ...
docs/site/Decorators.md
Outdated
|
||
For the simplest use case, you can leave the input of `@requestBody` empty, we automatically | ||
detect the type of `user`, generate the corresponding schema for it. | ||
And the default content type is set to be `application/json`. |
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.
The default content type ...
(no And
)
docs/site/Decorators.md
Outdated
|
||
So an example would be `@param.query.number('offset')`. | ||
You can find the specific usage in [Writing Controller methods](controller.md#writing-controller-methods) | ||
*We are supporting more `@requestBody` shorts in the future, track the feature in story* |
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.
shorts
?
docs/site/Decorators.md
Outdated
|
||
`@param` can be applied to method itself or specific parameters, it describes | ||
an input parameter of a Controller method. | ||
`@param` is applied to method parameters to generate OpenAPI parameter specification for it. |
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.
it
-> them
docs/site/Decorators.md
Outdated
|
||
`@param` can be applied to method itself or specific parameters, it describes | ||
an input parameter of a Controller method. | ||
`@param` is applied to method parameters to generate OpenAPI parameter specification for it. |
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.
Can we say controller method parameters
to be more explicit?
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.
section "parameter decorator" and "requestBody decorator" are under "Route decorators", which says they are for controllers. But true, the sections is far from their parent, I added keyword "controller" for both.
docs/site/Decorators.md
Outdated
} | ||
``` | ||
*About decorating models and the corresponding OpenAPI schema, please check* | ||
*[model decorators](#model-decorators)* |
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.
missing .
for the end of the sentence.
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.
Round 2 let's go
docs/site/Decorators.md
Outdated
``` | ||
|
||
or | ||
Writing the whole parameter specification is tedious, we've created shortcuts to define |
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.
... tedious, so we've ...
docs/site/Decorators.md
Outdated
Writing the whole parameter specification is tedious, we've created shortcuts to define | ||
the params with the pattern `@param.${in}.${type}(${name})`: | ||
|
||
- in: the parameter location, it can be one of the following values: `query`, `header`, `path`. |
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'm just going to list out incorrect usages of commas as COMMA for the rest of the review.
Commas can't be used to connect two separate thoughts; use either a semicolon (;)(if the sentences are similar enough in theme), a period (.), or reword the sentences so that they can be connected by commas.
COMMA
docs/site/Decorators.md
Outdated
|
||
- in: the parameter location, it can be one of the following values: `query`, `header`, `path`. | ||
- type: a [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types). | ||
- name: name of the parameter, it should be a `string`. |
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.
COMMA
docs/site/Decorators.md
Outdated
- type: a [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types). | ||
- name: name of the parameter, it should be a `string`. | ||
|
||
Take specifying a data from query as an example, a list of available shortcuts for can |
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.
COMMA
docs/site/Decorators.md
Outdated
decorator must be applied after an operation decorator. | ||
To learn more about TypeScript decorator composition, | ||
refer to [TypeScript Decorator Documentation](https://www.typescriptlang.org/docs/handbook/decorators.html) | ||
You can find the specific usage in [Writing Controller methods](controllers.md#writing-controller-methods) |
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.
You can find specific use cases in ...
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.
Also, the link is still dead
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.
@shimks I checked with Biniam, according to https://github.com/benbalter/jekyll-relative-links "controllers.md#writing-controller-methods" should be the correct format.
Do you have a valid example that I can look at?
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.
Oh, the PR might not have been rebased, that could be why the link is dead. I'd just advise to test out all links before pushing it upstream
docs/site/Decorators.md
Outdated
*To learn more about decorating models and the corresponding OpenAPI schema, please check* | ||
*[model decorators](#model-decorators).* | ||
|
||
Then a typical usage of `@requestBody` is specifying a parameter's type as the model class, |
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.
Grammatically, this sentence is incomplete. I expect a sentence listing out the condition using 'if' before the sentence using 'then'
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 get what you're trying to say now with this section. Here's how I would word it based on the context you've given:
In order to use 'requestBody', the parameter type it's decorating needs to have its model decorated with '@model' and '@property'
CODE EXAMPLE
This allows type information of the model to be visible to the spec generator and '@requestBody' can finally be used on the parameter:
CODE EXAMPLE
docs/site/Decorators.md
Outdated
|
||
You can also customize the generated `requestBody` specification in 3 ways: | ||
|
||
* add fields `description` and `required` |
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'd like to see a more succinct organization of use cases of requestBody
. Quoting from #1064 for inspiration:
single content-type
schema automatically generated with application/json
with or without optional properties
schema automatically generated with explicitly set content-type
with or without optional properties
schema set by user
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.
@shimks Sorry I don't quite agree with you here, the simplest example and 3 customized scenarios covered the following use cases:
schema automatically generated with application/json
with or without optional properties
schema automatically generated with explicitly set content-type
with or without optional properties
schema set by user
IMO there are three items can be customized:
- optional properties('description', 'required')
- content type
- schema spec
And that's how I classify the use cases.
docs/site/Decorators.md
Outdated
|
||
*Only one parameter can be decorated by `@requestBody` per controller method.* | ||
|
||
A typical OpenAPI requestBody spec contains properties `description`, `required`, and `content`: |
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.
docs/site/Decorators.md
Outdated
``` | ||
|
||
For the simplest use case, you can leave the input of `@requestBody` empty, | ||
since we automatically detect the type of `user`, and generate the corresponding schema for it. |
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.
no comma needed here
docs/site/Decorators.md
Outdated
} | ||
``` | ||
|
||
For the simplest use case, you can leave the input of `@requestBody` empty, |
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.
no comma between '@requestBody' empty
and 'since we automatically'
docs/site/Decorators.md
Outdated
- type: A [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types). | ||
- name: Name of the parameter. It should be a `string`. | ||
|
||
Take specifying a data from query as an example, a list of available shortcuts for can |
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 guess I'm confused as to what this sentence is trying to say. Did you possibly mean to say Taking
instead of Take
?
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.
For example, to specify data from a query, a list of available shortcuts can be found in [APIDocs](...)
docs/site/Decorators.md
Outdated
decorator must be applied after an operation decorator. | ||
To learn more about TypeScript decorator composition, | ||
refer to [TypeScript Decorator Documentation](https://www.typescriptlang.org/docs/handbook/decorators.html) | ||
You can find the specific usage in [Writing Controller methods](controllers.md#writing-controller-methods) |
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.
Oh, the PR might not have been rebased, that could be why the link is dead. I'd just advise to test out all links before pushing it upstream
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.
🛎 ROUND 3 🛎
docs/site/Decorators.md
Outdated
``` | ||
|
||
For the simplest use case, you can leave the input of `@requestBody` empty | ||
since we automatically detect the type of `user`, and generate the corresponding schema for it. |
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.
no comma
docs/site/Decorators.md
Outdated
*To learn more about decorating models and the corresponding OpenAPI schema, please check* | ||
*[model decorators](#model-decorators).* | ||
|
||
Then a typical usage of `@requestBody` is specifying a parameter's type as the model class, |
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 get what you're trying to say now with this section. Here's how I would word it based on the context you've given:
In order to use 'requestBody', the parameter type it's decorating needs to have its model decorated with '@model' and '@property'
CODE EXAMPLE
This allows type information of the model to be visible to the spec generator and '@requestBody' can finally be used on the parameter:
CODE EXAMPLE
docs/site/Decorators.md
Outdated
@@ -132,56 +132,181 @@ accordingly or do a composition of them: | |||
For more usage, refer to [Routing to Controllers](controllers.htm#routing-to-controllers) | |||
|
|||
### Parameter Decorator | |||
|
|||
Syntax: see [API document](https://github.com/strongloop/loopback-next/blob/master/packages/openapi-v3/src/parameter-decorator.ts#L18-L29) |
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.
Can we link to apidocs.loopback.io instead please as the line numbers may change over time.
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.
sorry replying late, there is a bug in tsdoc so I have to take this workaround, details see discussion:
#1145 (comment)
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.
@virkt25 BTW the fix for it is tracked in #584 (comment). I added a comment there to remind myself update doc after it's fixed.
docs/site/Decorators.md
Outdated
@@ -132,56 +132,185 @@ accordingly or do a composition of them: | |||
For more usage, refer to [Routing to Controllers](controllers.htm#routing-to-controllers) | |||
|
|||
### Parameter Decorator | |||
|
|||
Syntax: see [API documentation](https://github.com/strongloop/loopback-next/blob/master/packages/openapi-v3/src/parameter-decorator.ts#L18-L29) |
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.
Can we link to apidocs.loopback.io instead since line number may change over time
docs/site/Decorators.md
Outdated
|
||
Syntax: [`@param(paramSpec: ParameterObject)`]() | ||
Shortcut Pattern: `@param.${in}.${type}(${name})` |
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 would revert this line since the Shortcut Patter is introduced later in Line 170
docs/site/Decorators.md
Outdated
- type: A [common name of OpenAPI primitive data type](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#data-types). | ||
- name: Name of the parameter. It should be a `string`. | ||
|
||
Take specifying a data from query as an example, a list of available shortcuts for can |
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.
For example, to specify data from a query, a list of available shortcuts can be found in [APIDocs](...)
docs/site/Decorators.md
Outdated
Please note method level `@param` and parameter level `@param` are mutually exclusive, | ||
you can not mix and apply them to the same parameter. | ||
*The parameter location cookie is not supported yet, see* | ||
*https://github.com/strongloop/loopback-next/issues/997* |
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.
You don't need to end with *
on above line, it automatically carries the style over.
Please change the text for link to an actual link.
docs/site/Decorators.md
Outdated
- in: one of the following values: `query`, `header`, `path`, `formData`, `body` | ||
- type: one of the following values: `string`, `number`, `boolean`, `integer` | ||
- name: a `string`, name of the parameter | ||
Syntax: see [API documentation](https://github.com/strongloop/loopback-next/blob/master/packages/openapi-v3/src/request-body-decorator.ts#L20-L79) |
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.
Link to apidocs.loopback.io instead of code
b190bd2
to
5d02235
Compare
1f3c66d
to
24b9188
Compare
docs/site/Decorators.md
Outdated
} | ||
``` | ||
|
||
In order to use 'requestBody', the parameter type it's decorating needs to have its model decorated with '@model' and '@property' |
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.
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.
good catch!
docs/site/Decorators.md
Outdated
```ts | ||
import {User} from '../models/user' | ||
import {put} from '@loopback/rest' | ||
// in file controller.ts |
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.
nitpick: suggest our convention of /src/controllers/user.controller.ts
here
connect to #753
Checklist
npm test
passes on your machine(running)[ ] New tests added or existing tests modified to cover all changes[ ] Affected artifact templates inpackages/cli
were updated[ ] Affected example projects inpackages/example-*
were updated