Skip to content
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

[DO NOT REVIEW] feat: implement convention based boot #824

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"build:current": "lerna run --loglevel=silent build:current",
"pretest": "npm run build:current",
"test": "node packages/build/bin/run-nyc npm run mocha",
"test:clean": "npm run clean && npm run build:current && npm run test",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd switch the trigger to test:rebuild or something similar.

"mocha": "node packages/build/bin/select-dist mocha --opts test/mocha.opts \"packages/*/DIST/test/**/*.js\"",
"posttest": "npm run lint"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/boot/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
*.tgz
dist*
package
1 change: 1 addition & 0 deletions packages/boot/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
25 changes: 25 additions & 0 deletions packages/boot/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
Copyright (c) IBM Corp. 2017. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be Copyright (c) IBM Corp. 2017-2018. All Rights Reserved. now :-)

Node module: @loopback/boot
This project is licensed under the MIT License, full text below.

--------

MIT license

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
109 changes: 109 additions & 0 deletions packages/boot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# @loopback/boot

Boot package for LoopBack 4 to bootstrap convention based projects.

## Overview

This package provides the ability to bootstrap a LoopBack 4 project by
automatically automatically associating artifacts and configuration with an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2x automatically

application at runtime.

The package is currently consumed as a [Mixin](http://loopback.io/doc/en/lb4/Mixin.html).

It will automatically find all [Controller](http://loopback.io/doc/en/lb4/Controllers.html)
classes by searching through all files in `controllers` directory ending in
`.controller.js` and bind them to the application using `this.controller()`.

Other Mixins can support automatic Booting by overriding the `boot()` method.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the case of Booting to lowercase.

See example in [Advanced use](#advanced-use).

## Installation

```sh
npm i --save @loopback/boot
```

## Basic use

Using `@loopback/boot` is simple. It is a Mixin and should be added to your
application Class as shown below.

```ts
class BootedApplication extends BootMixin(Application) {
constructor(options?: ApplicationConfig) {
super(options);
}
}
```

### Configuration

Configuration options for boot can be set via the `ApplicationConfig` object
(`options`). The following options are supported:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am against adding boot configuration to ApplicationConfig because of the reasons outlined in #742:

Right now, Application constructor accepts a config/options object that mixes two very different concerns:

  1. The functionality/features the application should implement - what controllers it contains, what repositories are registered, etc. ("the assembly instructions")
  2. The configuration - what port to listen on, what database servers to connect to, etc.

IMO, the consumers of an application must not change its functionality and features (with the exception of enabling/disabling explicit feature flags) and therefore the config/options argument should contain only the configuration (port numbers, connection strings, feature flags).

Just think about it - how is the code loading an application supposed to know what directories the application stores its controller source files?

IMO, the configuration of file paths should be provided by the application class. The boot function should be pretty much self-contained, with the exception of arguments holding environment-specific configuration, e.g. environment variables or a flag distinguishing between dev/debug-like and production-like modes.


|Name|Default|Description|
|-|-|-|
|`boot.rootDir`|`process.cwd()`|The root directory for the project. All other paths are resolved relative to this path.|
|`boot.controllerDirs`|`controllers`|String or Array of directory paths to search in for controllers. Paths may be relative to `rootDir` or absolute.|
|`boot.controllerExts`|`controller.js`|String or Array of file extensions to consider as controller files.|

*Example*
```ts
app = new BootedApplication({
rest: {
port: 3000
},
boot: {
rootDir: '/absolute/path/to/my/app/root',
controllerDirs: ['controllers', 'ctrls'],
controllerExts: ['controller.js', '.ctrl.js'],
},
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs an end tag for the code above

## Advanced use

An extension developer can support the **boot** process by writing a mixin and
overriding the `boot()` function as shown below. If a user is using
`@loopback/boot`, it will automatically boot your artifact as well.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm conflicted about this approach. On the one hand, it gives us finer control over the boot order (which is something people have complained about with LB3), but it requires users to be careful about how they use mixins, and requires an awful lot of mixin chaining.

It could end up looking like this:

export class MyApplication extends ThirdScript(SecondScript(FirstScript(BootMixin(RestApplication)))) {
  // etc...
}

It's not a tremendously painful experience, but I think it'd be nice if we added some sugar to simplify this. If you check out the TypeScript handbook page on Mixins, it shows a pattern for applying an array of mixins without requiring users to extend things in this way.

Copy link
Contributor

@raymondfeng raymondfeng Jan 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I don't think Mixin is the right approach here. It has the same issue as Express app.use as the order of execution is controlled by the order how mixins are chained. I would rather use the extension point/extension pattern as we do in LoopBack 3.x.

Please note that a single boot() method is not going to work. There are multiple steps needed to discover/load/transform/resolve/start/stop various artifacts which can be cross-referenced. We need to allow each extensions to participate the boot process at different phases.

I strongly recommend that we follow a similar approach at https://github.com/strongloop/loopback-boot/tree/master. The main idea is as follows:

  1. The @loopback/boot will have an extension point for all booters as separate extensions. The extension point can find corresponding extensions from the context.

  2. Each booter can be registered using context bindings, such as booters.controller, booters.model, and booters.datasource.

  3. There will be a BootLifeCycle interface to define optional steps, such as:

export interface BootLifeCycle {
  discover?(...);
  load?(...);
  transform?(...);
  resolve?(...);
  start?(...);
  stop?(...);
}

A booter can choose to implement what methods to implement.

  1. app.boot will give control to the boot extension point, which in turn coordinates the registered booters to boot artifacts in the order of life cycle events.

  2. app.boot have options to control what booters should be involved and what life cycle phases should be triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments Raymond. Looking for some clarification.

  1. My understanding of loopback-boot right now is that it runs scripts found in a boot folder alphabetically / in a given order by requiring them. We can accomplish the same using the Mixin by running scripts in a boot folder. This task was just about discovering and binding artifacts.

  2. I sort of see the value of having different functions for discover/load/transform/etc. but if the purpose of boot is to find and load artifacts than I can imagine a single function being able to do that as a extension developer can write the boot function to do what they want.

  3. I'm not sure I understand the purpose of certain phases that are proposed such as resolve/start/stop. Resolve in particular is something that I think is better handled by Dependency Injection.

With @kjdelisle 's change, you can control Mixin order by passing them in via an array.

Just trying to understand the value add of having phases for boot vs. just booting by discovering artifacts via Mixin followed by running scripts in a boot folder like current loopback-boot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of loopback-boot right now is that it runs scripts found in a boot folder alphabetically / in a given order by requiring them. We can accomplish the same using the Mixin by running scripts in a boot folder. This task was just about discovering and binding artifacts.

No. Boot script is just one of the artifact types we support in LoopBack 3.x. There are different plugins for model definitions, data sources, middleware, components, and boot scripts. Each plugin is responsible for loading/compiling/executing its own kind of artifact.

I sort of see the value of having different functions for discover/load/transform/etc. but if the purpose of boot is to find and load artifacts than I can imagine a single function being able to do that as a extension developer can write the boot function to do what they want.

One pass is usually not good enough to handle artifacts that have references to other ones. For example, model Customer has a hasMany relation to model Order and Order has a belongsTo relation to Customer. A single boot() for models cannot build the runtime representation of the relations between the two models .

I'm not sure I understand the purpose of certain phases that are proposed such as resolve/start/stop. Resolve in particular is something that I think is better handled by Dependency Injection.

Resolve - connect multiple artifacts, for example, set up the target model reference for a relation.
Start - start an artifact, for example, a server instance or a database connection
Stop - stop an artifact, for example, a server instance or a database connection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. But shouldn't references be resolved by Dependency Injection when needed if boot sets up bindings properly which is what it should be doing ...?

And I see the mixins folder on loopback-boot. So it controls loading of artifacts by calling the start() for each mixin?

I'm also not sure what it means to start / stop an artifact ... or how stop will even be called?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea sorry I meant plugins not mixins.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raymondfeng Your proposed changes are out of scope for the task. If this is something we want, then we should create a new task, groom it and poker it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the approach taken by the PR is architecturally flawed. I'm not sure if we really want to land it as is. The boot module is fundamental to the declarative support for LoopBack 4. I think we need to start with the right path from the beginning.

I don't mind starting with a single boot() implemented by individual booters, but mixin is not suitable for this use case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great discussion!

Let me clarify few possible confusions I anticipate from the discussion above before I post my opinions.

In LoopBack 3.x, there are two versions of loopback-boot. 2.x is what almost everybody uses today, because it's the latest version published to npmjs.org and used by our CLI tooling when scaffolding a new project. It has rather simple and less flexible design, the order of individual boot steps is hard coded and there are no extension points for components. Then there is the upcoming new version 3.0 living in the master branch containing a rewrite by @raymondfeng and @davidcheung, which provides much more flexible design with extension points for components. I believed this version was never published (see strongloop/loopback-boot#241), but as I checked the master branch today, there are apparently some 3.x versions published to npmjs.org. Because the release process of the new semver-major version was not done properly, these 3.x versions are effectively hidden by 2.x updates. What a mess! But let's not digress - I just wanted to clarify the state of loopback-boot to prevent confusion created by people (e.g. from our community) reading the discussion above.

My understanding of loopback-boot right now is that it runs scripts found in a boot folder alphabetically / in a given order by requiring them. We can accomplish the same using the Mixin by running scripts in a boot folder. This task was just about discovering and binding artifacts.

This is definitely the case in loopback-boot 2.x. I don't know how the new 3.x version handles boot scripts.

And I see the mixins folder on loopback-boot. So it controls loading of artifacts by calling the start() for each mixin?

Even though this has been clarified that @virkt25 meant plugins, not mixins`, I'd like to note that in LoopBack 3.x, we have a concept of Model Mixins that allow users to apply a mixin to a model class, and at least loopback-boot 2.x does load and execute such mixin scripts.


[@kjdelisle] @raymondfeng Your proposed changes are out of scope for the task. If this is something we want, then we should create a new task, groom it and poker it.
[@raymondfeng] IMO, the approach taken by the PR is architecturally flawed. I'm not sure if we really want to land it as is. The boot module is fundamental to the declarative support for LoopBack 4. I think we need to start with the right path from the beginning.
I don't mind starting with a single boot() implemented by individual booters, but mixin is not suitable for this use case.

I agree with both of you.

IMO, we should not aim for a fully flexible bootstrapper in this first iteration targeting our MVP milestone. For MVP, it's ok to have a hard-coded bootstrapper with no (or very little) extension points.

At the same time, I agree that mixin composition is not a good architecture for our bootstrapper and therefore we should not pursue it even for MVP, because upgrading from mixin composition to a different architecture would be too disruptive.

TBH, I find Raymond's BootLifeCycle with pre-defined steps as suboptimal too. Assuming each of the steps like discover and load are run fully for all components before the next step is executed (i.e. run discover for all components and only then run load for all components), then how are we going to handle the situation when ComponentA needs the output of load step from ComponentB before it can discover its own artifacts? On the other hand, if entire BootLifeCycle is run for each component before the next component is booted (i.e. run discover and load for ComponentA, then for ComponentB), then how are we going to handle the situation when ComponentA needs to run some task after ComponentB has loaded & transformed all its artifacts, but before ComponentB attempts to resolve them?

I am proposing to explore our "Sequence" design pattern we are using for composing request handling middleware and see if and how it can be applied to the boot process too. I see two benefits of such approach:

  • By reusing the same concept for both request handling logic and bootstrapping, users can apply their existing knowledge in multiple places. The framework should be easier to learn as a result.

  • Sequence makes the flow control explicit, easy to understand and modify.

Right now, the boot process (both master (3.x) and 2.x branches of loopback-boot for LoopBack 3.x) is very opaque. The current 2.x branch does not allow any customization of boot steps - see the hard-coded algorithm in executor.js. The new version in master (which hasn't been released for public use yet - see strongloop/loopback-boot#241) seems to promise more flexibility in that matter, but my knowledge of that new codebase is not sufficient to tell how well we are delivering on that promise.

Let me post a mock-up boot sequence to illustrate my idea:

type EnvironmentVariables = {[name: string]: string};

interface BootOptions {
  projectRoot: string;
  env?: EnvironmentVariables;
}

interface AppBootSequence<App> {
  boot(app: App, options: BootOptions);
}

class MyBootSequence implements AppBootSequence<MyApplication> {
   /* constructor receiving dependencies - I am leaving that out for brevity */

  public boot(app: MyApplication, options: BootOptions) {
    this.setupDataSources(app, options);
    this.setupRepositoriesAndModels(app, options);
    this.resolveModelRelations(app, options);
    this.setupControllers(app, options);
    this.start(app, env);
  }
}

// setupDataSources step provided by @loopback/repository or similar
setupDataSources(app, options) {
  const controllersDir = path.resolve(
    options.projectRoot,
    'controllers' || options.controllersDir);

  const controllerExts = options.controllerExts || 'controller.js';

  bootClassArtifacts(app, controllersDir, controllerExts, a => app.controller(a));
}

// usage in Application
// this code should be scaffolded by our CLI tooling
// together with MyBootSequence template
class MyApplication {
  constructor(/*...*/) {
    this.bind('bootstrapper').toClass(MyBootSequence)
  }

  async boot(env: EnvironmentVariables) {
    const options: BootOptions = {
      projectRoot: __dirname,
      env: env,
    }
    const sequence = await this.get('bootstrapper');
    await sequence.boot(app, options);
  }
}

Since this design is similar with REST Sequence Handler, I would expect it should be reasonably easy to leverage the extension point #657 and action composition #713 features that @raymondfeng is working on, to get an even more flexible solution.

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me expand a bit on why I am against mixin composition.

Consider ComponentA and ComponentB, where ComponentA needs to run it's boot function only after ComponentB has already booted. This requires ComponentB to call super.boot() before running its own boot steps and the application developer to compose mixins in the correct order. As a result, neither the ComponentB nor the application developer are fully in control of the order in which individual boot steps are invoked and the correct order of boot steps requires cooperation of multiple parts of the codebase. I see that as a sign of Low Cohesion & High Coupling, which typically makes code more difficult to understand and maintain.

(I admit my solution proposed above does not address this problem entirely, because in the Sequence design, individual sequence steps have no control of the order in which they are executed. OTOH, the action composition technique proposed by Raymond should address that part in mostly backwards-compatible way.)


```ts
function TestMixin<T extends Constructor<any>>(superClass: T) {
return class extends superClass {
constructor(...args: any[]) {
super(...args);
}

async boot() {
// Your custom config
const repoDir = this.options.boot.repoDir || 'repositories';
// We call the convenience method to boot class artifacts
await this.bootClassArtifacts(repoDir, 'repository.js', 'testMixinRepo');

// IMPORTANT: This line must be added so all other artifacts can be booted
// automatically and regardless of the order of the Mixins.
if (super.boot) await super.boot();
}
};
}
```

## Related resources

**Coming Soon** Link to Boot Docs.

## Contributions

- [Guidelines](https://github.com/strongloop/loopback-next/wiki/Contributing#guidelines)
- [Join the team](https://github.com/strongloop/loopback-next/issues/110)

## Tests

run `npm test` from the root folder.

## Contributors

See [all contributors](https://github.com/strongloop/loopback-next/graphs/contributors).

## License

MIT
14 changes: 14 additions & 0 deletions packages/boot/docs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"content": [
"index.ts",
"src/index.ts",
"src/types.ts",
"src/utils.ts",
"src/mixins/boot.mixin.ts"
],
"codeSectionDepth": 4,
"assets": {
"/": "/docs",
"/docs": "/docs"
}
}
6 changes: 6 additions & 0 deletions packages/boot/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2013,2017. All Rights Reserved.
// Node module: @loopback/boot
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

export * from './dist/src';
6 changes: 6 additions & 0 deletions packages/boot/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2013,2017. All Rights Reserved.
// Node module: @loopback/boot
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

module.exports = require('./dist/src');
6 changes: 6 additions & 0 deletions packages/boot/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2013,2017. All Rights Reserved.
// Node module: @loopback/boot
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

export * from './src';
46 changes: 46 additions & 0 deletions packages/boot/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"name": "@loopback/boot",
"version": "4.0.0-alpha.1",
"description": "Boot handler for LoopBack 4 apps",
"engines": {
"node": ">=8"
},
"main": "index.js",
"scripts": {
"acceptance": "mocha --opts ../../test/mocha.opts dist/test/acceptance/**/*.js",
"build": "lb-tsc",
"build:current": "lb-tsc",
"build:apidocs": "lb-apidocs",
"clean": "lb-clean loopback-boot*.tgz dist package api-docs",
"prepare": "npm run build",
"pretest": "npm run clean && npm run build",
"test": "mocha --opts ../../test/mocha.opts dist/test/",
"unit": "mocha --opts ../../test/mocha.opts dist/test/unit/",
"verify": "npm pack && tar xf loopback-boot*.tgz && tree package && npm run clean"
},
"author": "IBM",
"license": "MIT",
"keywords": ["LoopBack", "Boot"],
"files": [
"README.md",
"index.js",
"index.d.ts",
"dist/src",
"api-docs",
"src"
],
"repository": {
"type": "git",
"url": "https://github.com/strongloop/loopback-next.git"
},
"devDependencies": {
"@loopback/build": "^4.0.0-alpha.7",
"@loopback/core": "^4.0.0-alpha.26",
"@loopback/rest": "^4.0.0-alpha.15",
"@loopback/testlab": "^4.0.0-alpha.17"
},
"dependencies": {
"@loopback/context": "^4.0.0-alpha.23",
"debug": "^3.1.0"
}
}
8 changes: 8 additions & 0 deletions packages/boot/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright IBM Corp. 2013,2017. All Rights Reserved.
// Node module: @loopback/boot
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

export * from './mixins/boot.mixin';
export * from './utils';
export * from './types';
Loading