Skip to content

Boot the app asynchronously #172

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.

Sign up for GitHub

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

Merged
superkhau merged 1 commit into from
Feb 24, 2015
Merged

Boot the app asynchronously #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion templates/components/api-server/component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ template.package = {
"compression": "^1.0.3",
"errorhandler": "^1.1.1",
"loopback": "^2.8.0",
"loopback-boot": "^2.4.0",
"loopback-boot": "^2.6.5",
"loopback-datasource-juggler": "^2.7.0",
"serve-favicon": "^2.0.1"
},
Expand Down
17 changes: 9 additions & 8 deletions templates/components/api-server/template/server/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ var boot = require('loopback-boot');

var app = module.exports = loopback();

// Bootstrap the application, configure models, datasources and middleware.
// Sub-apps like REST API are mounted via boot scripts.
boot(app, __dirname);

app.start = function() {
// start the web server
return app.listen(function() {
Expand All @@ -15,7 +11,12 @@ app.start = function() {
});
};

@bajtos bajtos Feb 20, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We must preserve the event started, it's used by server/boot/explorer.js to print a message with Explorer URL, but only after the app was started.

@superkhau superkhau Feb 20, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. It's different from booted, adding back.

// start the server if `$ node server.js`
if (require.main === module) {
app.start();
}
// Bootstrap the application, configure models, datasources and middleware.
// Sub-apps like REST API are mounted via boot scripts.
boot(app, __dirname, function(err) {
if (err) throw err;

// start the server if `$ node server.js`
if (require.main === module)
app.start();
});
13 changes: 13 additions & 0 deletions test/end-to-end.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,19 @@ describe('end-to-end', function() {
done(err);
});
});

it('emits the `booted` event when booting is complete', function(done) {
var src = FIXTURES + '/async.js';
var dest = SANDBOX + '/server/boot/async.js';
fs.copySync(src, dest);
delete require.cache[require.resolve(SANDBOX)];
var app = require(SANDBOX);
app.on('booted', function() {
expect(app.asyncBoot, 'app.asyncBoot').to.be.true();
done();
});
// the test will time out if `booted` is not emitted
});
});

describe('autoupdate', function() {
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/async.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = function(app, cb) {
app.asyncBoot = typeof cb === 'function';

@bajtos bajtos Feb 20, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This does not really verify that the script are executed asynchronously. You should check that the boot process waits until the async scripts finished.

I think you should rework the two new tests and merge them into one.

// async.js
module.exports = function(app, cb) {
  app.asyncBoot = typeof cb === 'function';
  process.nextTick(cb);
}

// in the test
var app = require(...);
expect(app.booting, 'app.booting').to.be.true();
app.on('booted', function() {
  expect(app.asyncBoot, 'app.asyncBoot').to.be.true();
  done(); 
});
// the test will time out if `booted` is not emitted

Thoughts?

@superkhau superkhau Feb 21, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree. Merging into one simple script instead.

process.nextTick(cb);
};
1 change: 1 addition & 0 deletions test/support.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ expectValueInJSONFile = function(file, propertyPath, val) {
expect(obj).to.have.deep.property(propertyPath, val);
}

FIXTURES = path.resolve(__dirname, 'fixtures/');
SANDBOX = path.resolve(__dirname, 'sandbox/');

// tell the workspace to load files from the sandbox
Expand Down