fix(example-getting-started): use RestApplication

[shimks]

• application.ts now extends RestApplication instead of Application
• also added missing imports (@loopback/openapi-spec for todo.model.ts)
• connected to [Docs] Update documentation and examples to use RestApplication #846

Checklist

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• Related API Documentation was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in packages/example-* were updated

[virkt25]

I have this in the boot PR but it's a great idea to land this as a separate change. Left a few comments.

[virkt25]

You no longer need RestComponent.

[virkt25]

Why is this being added? (Don't see it being used in the package).

[shimks]

It seems to be used in todo.model.ts, but the compiler didn't complain about the package not being a dependency though, it's weird. It's entirely unrelated to migrating to RestApplication btw.

[jannyHou]

I think it's because this example still uses top-down approach to define the schema for a model.

[virkt25]

It's weird indeed that it wasn't complaining about the dependency being missing before. Makes me wonder if something is broken / needs to be investigated with out build process. (Not a blocker for this PR ~ Great catch!)

[bajtos]

The changes look good to me. There is more cleanup related to RestApplication - code setting up per-server configuration like server.route, server.sequence, etc. should be moved out of app.start to app constructor now.

See e.g. pages/en/lb4/Routes.md:

const app = new RestApplication();
// ...
(async function start() {
  const server = await app.getServer(RestServer);
  const route = new Route('get', '/', spec, greet);
  server.route(route);
  await app.start();
})();

Proposed update:

const app = new RestApplication();
const route = new Route('get', '/', spec, greet);
app.route(route);
// ...
(async function start() {
  await app.start();
})();

We need to check all places that contain REST-related configuration in app.start(). It's ok for me to make these changes in a follow-up pull request. Small incremental pull requests FTW!

[shimks]

@bajtos I'll work on a PR for your proposal at loopback.io. Good proposal by the way!

[shimks]

@bajtos On second thought, I've noticed that almost all examples that set up rest config have it done in the async start function. Do we want to make this change for sure?

For example, our own CLI template sets up the config inside the start function

async start() {
    const server = await this.getServer(RestServer);
    server.sequence(MySequence);
    const port = await server.get('rest.port');
    console.log(`Server is running at http://127.0.0.1:${port}`);
    console.log(`Try http://127.0.0.1:${port}/ping`);
    return await super.start();
  }

This issue may need more discussion and I'd feel it might be better dealt in a separate issue.

[bajtos]

@bajtos On second thought, I've noticed that almost all examples that set up rest config have it done in the async start function. Do we want to make this change for sure?

For example, our own CLI template sets up the config inside the start function

async start() {
    const server = await this.getServer(RestServer);
    server.sequence(MySequence);
    const port = await server.get('rest.port');
    console.log(`Server is running at http://127.0.0.1:${port}`);
    console.log(`Try http://127.0.0.1:${port}/ping`);
    return await super.start();
  }

This issue may need more discussion and I'd feel it might be better dealt in a separate issue.

IIRC, the original code snippets were configuring sequence & routes in the constructor. This setup code was moved to async start function after @kjdelisle implemented support for multiple REST servers. I was never happy about that change to be honest. For example, it means that code inspecting application's OpenAPI spec (think of lb export-api-def) cannot see all routes until the application is started. Now that we have RestApplication, there is no need to use that suboptimal workaround and we can revert back to the original version.

I am ok to leave those changes out of scope of this PR, but I think they are part of #846. Let me leave a comment there too.

[bajtos]

See #968