Declaratively load loopback.rest and loopback-explorer

[strongloop]

Declaratively load loopback.rest and loopback-explorer

[raymondfeng]

I would also need to access the app instance either via the parameter injection or set by boot to the factory/handler instance.

[hacksparrow]

Let's address the issue "one feature at a time". I think 2. Allowing the middleware to be optional is the most straightforward one, so let's give it the first shot.

So @bajtos what are the requirements for 2? Here is what I am assuming:

1. If the middleware is not resolvable, don't throw and resume loading the rest of the middleware. Also, print the optional message to stdio.
2. If the middleware is resolvable, load it like other middleware.

Please confirm or correct.

[bajtos]

1. If the middleware is not resolvable, don't throw and resume loading the rest of the middleware. Also, print the optional message to stdio.
2. If the middleware is resolvable, load it like other middleware.

@hacksparrow that's perfect 👍

Let's address the issue "one feature at a time".

+1 I'd add "one pull request per feature", but feel free to use whatever workflow that works best for you.

[hacksparrow]

@bajtos I have implemented a way to to pass app as a parameter to middleware via middleware.json. app can be referenced as ${app}.

middleware.json

"routes": {
  "./middleware/hello": {
    "params": ["${app}", {"app": "${app}"}]
  }
}

How does the convention ${app} look like to you?

[hacksparrow]

@bajtos also, for explorer to work seamlessly as a self-contained optional middleware, the middleware config should be the following, if I am not wrong:

{
  "routes": {
    "loopback-explorer": {
      "optional": "Run `npm install loopback-explorer` to enable the LoopBack explorer.",
      "params":  ["${app}", {
          "basePath": "${restApiRoot}"
        }
      ]
   }
}

[hacksparrow]

and of course, explorer has to take over the responsibility of doing what server/boot/explorer.js is doing currently.

  var restApiRoot = server.get('restApiRoot');

  var explorerApp = explorer(server, { basePath: restApiRoot });
  server.use('/explorer', explorerApp);
  server.once('started', function() {
    var baseUrl = server.get('url').replace(/\/$/, '');
    console.log('Browse your REST API at %s%s', baseUrl, explorerApp.mountpath);
  });

[hacksparrow]

@bajtos the implementation for parsing ${app}, and its test is sitting on a branch on my fork - https://github.com/hacksparrow/loopback-boot/tree/feature/app-reference.

I'd like to open a PR for it, once the open PR for making middleware optional is merged.

[bajtos]

@hacksparrow I am afraid your proposal above goes against the original intention of this issue, which is to "Rework the explorer so that it can get the app object via req.app on the first request, in the same way as loopback.rest." I personally think it's a bad practice for middleware to expect the app object in factory arguments, it should get the app it's mounted on via req.app.

However, per the discussion in strongloop/loopback-component-explorer#21, I believe the agreed way forward is to rework loopback-explorer into a component. I'll take a close look on this tomorrow (CEST).

[hacksparrow]

Thanks for the feedback @bajtos, will change accordingly.

[hacksparrow]

@bajtos per this example:

{
  "routes": {
    "loopback#rest": {
      "paths": ["${restApiRoot}"]
    }
}

We will now call loopback.rest() directly via a configuration in middleware.json, instead of using server/boot/rest-api.js, and therefore make the necessary changes to loopback/server/middleware/rest.js to enable this, right?

[bajtos]

We will now call loopback.rest() directly via a configuration in middleware.json, instead of using server/boot/rest-api.js

Yes please.

therefore make the necessary changes to loopback/server/middleware/rest.js to enable this, right?

I am not aware of any changes that are necessary, could you please elaborate little bit more?

[bajtos]

therefore make the necessary changes to loopback/server/middleware/rest.js to enable this, right?

Did you mean implementing necessary changes to support "paths": ["${restApiRoot}"]? That should be implemented in loopback-boot's lib/executor.js

[hacksparrow]

Thanks for the info @bajtos.

Another thing; what would be the interface for doing something like this?

var websitePath = require('path').resolve(__dirname, '../client');
app.use(loopback.static(websitePath));

[bajtos]

var websitePath = require('path').resolve(__dirname, '../client');
app.use(loopback.static(websitePath));

Assuming __dirname is pointing to the same directory where server/middleware.json is, then you can achieve the same result using the following entry in server/middleware.json file:

{
  "files": {
    "loopback#static": {
      "params": "$!../client"
    }
  }
}

[hacksparrow]

@bajtos opened a PR to declaratively load loopback.rest - #149. However, there is a bunch of build errors triggered by some pre-existing code, I can fix it if I get the go-ahead. Please review and let me know.

[hacksparrow]

Also, are we not supporting const yet?

[hacksparrow]

@bajtos is there a reason why loopback#rest takes an array of paths while loopback-explorer takes a single mountPath?

[crandmck]

@bajtos Can this be closed with the merging of #149?

If not, what are the remaining items to complete before this can be closed?

[hacksparrow]

@crandmck component-config.json should parse ${var} too. I have gotten it to parse, but still working on a way for explorer to be loaded declaratively and replicate its current behavior.

With explorer loading as intended, we can close this.

[bajtos]

@crandmck as @hacksparrow wrote, we need to land one more change to loopback-boot and then rework the example app to load the explorer component declaratively.

@hacksparrow don't worry about reproducing exactly same behaviour. The important part is to print the explorer URL on start. The support for explorer being an optional dependency can be dropped and explorer can become a proper (required) dependency.

[hacksparrow]

@bajtos @crandmck I guess we can close this now?