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.

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

feat: app.close return a promise #19

Merged
merged 3 commits into from
Oct 19, 2016
Merged

feat: app.close return a promise #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4fd93ae
feat: app.close return a promise
popomore Oct 16, 2016
a83362a
Merge branch 'master' into close-promise
popomore Oct 19, 2016
d679851
Merge branch 'master' into close-promise
popomore Oct 19, 2016
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
12 changes: 10 additions & 2 deletions lib/egg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,19 @@ class EggCore extends KoaApplication {
/**
* close all listeners
* @member {Function}
* @return {Promise} promise
* @since 1.0.0
*/
close() {
this.emit('close');
this.removeAllListeners();
return new Promise((resolve, reject) => {
try {
Copy link
Member

Choose a reason for hiding this comment

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

意义在哪里呢？我看现在这是一个同步的方法啊？

Copy link
Member

Choose a reason for hiding this comment

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

为什么要return promise

Copy link
Member Author

Choose a reason for hiding this comment

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

可能有异步的情况。

比如测试的时候可以 after(() => app.close());

Copy link
Member

Choose a reason for hiding this comment

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

现在也可以这样写啊，close 是同步的

Copy link
Member

Choose a reason for hiding this comment

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

emit close 和 removeAllListeners 都是同步方法

Copy link
Member Author

Choose a reason for hiding this comment

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

可能会有异步的，继承后保持 api 一致

Copy link
Member Author

Choose a reason for hiding this comment

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

close() {
  return super
    .close()
    .then(resolve => {
      // close sth
      resolve();
    });
}

this.emit('close');
this.removeAllListeners();
resolve();
} catch (err) {
reject(err);
}
});
}

/**
Expand Down
20 changes: 19 additions & 1 deletion test/egg.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ describe('test/egg.test.js', () => {

describe('app.close()', () => {
let app;
afterEach(() => app.close());

it('should emit close event before exit', () => {
app = utils.createApp('close');
Expand All @@ -157,5 +156,24 @@ describe('test/egg.test.js', () => {
app.close();
called.should.equal(true);
});

it('should return a promise', done => {
app = utils.createApp('close');
const promise = app.close();
promise.should.instanceof(Promise);
promise.then(done);
});

it('should throw when close error', done => {
app = utils.createApp('close');
mm(app, 'removeAllListeners', () => {
throw new Error('removeAllListeners error');
});
app.close().catch(err => {
err.message.should.eql('removeAllListeners error');
done();
});
});
});

});