-
Notifications
You must be signed in to change notification settings - Fork 383
Move RestServer config out of async start
#621
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,13 +32,17 @@ tasks as a part of your setup: | |
| import {Application} from '@loopback/core'; | ||
| import {RestComponent, RestServer} from '@loopback/rest'; | ||
| import {SamoflangeController, DoohickeyController} from './controllers'; | ||
| import {WidgetApi} from './apidef/'; | ||
|
|
||
| export class WidgetApplication extends Application { | ||
| constructor() { | ||
| // This is where you would pass configuration to the base constructor | ||
| // (as well as handle your own!) | ||
| super(); | ||
| super({ | ||
| rest: { | ||
| port: 8080 | ||
| } | ||
| }); | ||
|
|
||
| const app = this; // For clarity. | ||
| // You can bind to the Application-level context here. | ||
| // app.bind('foo').to(bar); | ||
|
|
@@ -47,17 +51,6 @@ export class WidgetApplication extends Application { | |
| app.controller(DoohickeyController); | ||
| } | ||
|
|
||
| async start() { | ||
| // This is where you would asynchronously retrieve servers, providers and | ||
| // other components to configure them before launch. | ||
| const server = await app.getServer(RestServer); | ||
| server.bind('rest.port').to(8080); | ||
| server.api(WidgetApi); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change in the context of the original example. The original example assumed api-first approach, where Since api-first support is out of scope of our upcoming release, I am ok to rework this example to code-first approach, where the REST API is described on controller classes/methods using decorators like In other words, this change is fine with me too 👍 |
||
| // The superclass start method will call start on all servers that are | ||
| // bound to the application. | ||
| return await super.start(); | ||
| } | ||
|
|
||
| async stop() { | ||
| // This is where you would do whatever is necessary before stopping your | ||
| // app (graceful closing of connections, flushing buffers, etc) | ||
|
|
@@ -191,6 +184,15 @@ export class MyApplication extends RestApplication { | |
| ## Tips for application setup | ||
| Here are some tips to help avoid common pitfalls and mistakes. | ||
|
|
||
| ### Extend from `RestApplication` when using `RestServer` | ||
| If you want to use `RestServer` from our `@loopback/rest` package, we recommend you extend | ||
| `RestApplication` in your app instead of manually binding `RestServer` or | ||
| `RestComponent`. `RestApplication` already uses `RestComponent` and makes | ||
| useful functions in `RestServer` like `handler()` available at the app level. | ||
| This means you can call these `RestServer` functions to do all of your | ||
| server-level setups in the app constructor without having to explicitly retrieve | ||
| an instance of your server. | ||
|
|
||
| ### Use unique bindings | ||
| Use binding names that are prefixed with a unique string that does not overlap | ||
| with loopback's bindings. As an example, if your application is built for | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,9 +289,17 @@ _.merge(spec, CategoryAPI); | |
| export default spec; | ||
| ``` | ||
|
|
||
| You can then bind the full spec to the application using `server.spec()`. This is done on the server level, because each server instance can expose a different (sub)set of API. | ||
| You can then bind the full spec to the application using `app.api()`. | ||
| This works well for applications with a single REST server, because | ||
| there is only one API definition involved. | ||
|
|
||
| You also need to associate the controllers implementing the spec with the app using `app.controller(GreetController)`. This is not done on the server level because a controller may be used with multiple server instances, and types! | ||
| If you are building an application with multiple REST servers, | ||
| where each server provides a different API, then you need | ||
| to call `server.api()` instead. | ||
|
|
||
| You also need to associate the controllers implementing the spec with the app | ||
| using `app.controller(GreetController)`. This is not done on the server level | ||
| because a controller may be used with multiple server instances, and types! | ||
|
|
||
| ```ts | ||
| // application.ts | ||
|
|
@@ -302,20 +310,18 @@ import { ProductController, DealController, CategoryController } from "./control | |
| export class YourMicroservice extends RestApplication { | ||
|
|
||
| constructor() { | ||
| super(); | ||
| super({ | ||
| rest: { | ||
| port: 3001 | ||
| } | ||
| }); | ||
| const app = this; | ||
|
|
||
| app.controller(ProductController); | ||
| app.controller(DealController); | ||
| app.controller(CategoryController); | ||
|
|
||
| } | ||
| async start() { | ||
| const server = await app.getServer(RestServer); | ||
| // inject your spec here! | ||
| server.api(spec); | ||
| server.bind("rest.port").to(3001); | ||
| await super.start(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we still need to call
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @b-admike I think in this case it's enough to just inherit |
||
| //inject your spec | ||
| app.api(spec); | ||
| } | ||
| // etc... | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,7 +79,6 @@ application. | |
| import {RestApplication, RestServer, Route} from '@loopback/rest'; | ||
| import {OperationObject} from '@loopback/openapi-spec'; | ||
|
|
||
| const app = new RestApplication(); | ||
| const spec: OperationObject = { | ||
| parameters: [{name: 'name', in: 'query', type: 'string'}], | ||
| responses: { | ||
|
|
@@ -95,12 +94,11 @@ function greet(name: string) { | |
| return `hello ${name}`; | ||
| } | ||
|
|
||
| (async function start() { | ||
| const server = await app.getServer(RestServer); | ||
| const route = new Route('get', '/', spec, greet); | ||
| server.route(route); | ||
| await app.start(); | ||
| })(); | ||
| const app = new RestApplication(); | ||
| const route = new Route('get', '/', spec, greet); | ||
| app.route(route); // attaches route to RestServer | ||
|
|
||
| app.start(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't app.start() returning a promise and thus should be called with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that's a myth.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification Raymond. We don't really need to show |
||
| ``` | ||
|
|
||
| ### Using Route decorators with controller methods | ||
|
|
@@ -140,9 +138,7 @@ const app = new RestApplication(); | |
|
|
||
| app.controller(GreetController); | ||
|
|
||
| (async function start() { | ||
| await app.start(); | ||
| })(); | ||
| app.start(); | ||
| ``` | ||
|
|
||
| ## Invoking operations using Routes | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,20 +86,16 @@ class MySequence extends DefaultSequence { | |
| } | ||
| ``` | ||
|
|
||
| In order for LoopBack to use your custom sequence, you must register it on any | ||
| applicable `Server` instances before starting your `Application`: | ||
| In order for LoopBack to use your custom sequence, you must register it | ||
| before starting your `Application`: | ||
|
|
||
| ```js | ||
| import {RestApplication, RestServer} from '@loopback/rest'; | ||
|
|
||
| const app = new RestApplication(); | ||
| app.sequence(MySequencce); | ||
|
|
||
| // or | ||
| (async function start() { | ||
| const server = await app.getServer(RestServer); | ||
| server.sequence(MySequence); | ||
| await app.start(); | ||
| })(); | ||
| app.start(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
| ``` | ||
|
|
||
| ## Advanced topics | ||
|
|
||
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.
IMO, you should extend
RestApplicationhere. The only reason why you would not want to, would be if your application was exposing multiple REST servers, which is not the case AFAICT.This will also allow you to get rid of
app.component(RestComponent)below.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 only problem I have with that is the page the docs is about:
Application. IMO an example shown in this page should be vanillaApplicationinstead ofRestApplication. What do you think?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.
Fair enough, let's keep using
Applicationthen 👍Could you perhaps extend Tips for application setup to mention
RestApplication+ when and why to use it? It's ok to leave that out of this pull request though.