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

feat: app.js/agent.js support async function #18

Merged
merged 3 commits into from
Oct 24, 2016
Merged

Conversation

popomore
Copy link
Member

@popomore popomore commented Oct 17, 2016

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

custom app/agent

Description of change

You can use async function to write async code, never readyCallback.

module.exports = async function(app) {
  await doAsync();
};

Or use promise wrap on old node

const co = require('co');
module.exports = co.wrap(function* (app) {
  yield doAsync();
});

Closes eggjs/egg#51

@@ -157,6 +158,8 @@ class EggCore extends KoaApplication {
}).on('ready_timeout', id => {
this.console.warn('[egg:core:ready_timeout] 10 seconds later %s was still unable to finish.', id);
});

this.ready(() => debug('egg emit ready, application started'));
Copy link
Member

Choose a reason for hiding this comment

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

改成 this.logger.debug 吧

Copy link
Member Author

Choose a reason for hiding this comment

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

debug 调试起来方便吧

Copy link
Member

Choose a reason for hiding this comment

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

但是对项目开发者不方便。logger.debug 也可以通过 环境变量打开。

Copy link
Member Author

Choose a reason for hiding this comment

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

我们把 logger.debug 换成 node 的 debug?现在 debug 不能用环境变量打开,没有作用域,而且不能应用于底层模块。还有不能单独开启 debug,开启后 info 以上都会开启。

@codecov-io
Copy link

codecov-io commented Oct 19, 2016

Current coverage is 99.84% (diff: 100%)

Merging #18 into master will increase coverage by <.01%

@@             master        #18   diff @@
==========================================
  Files            16         16          
  Lines           622        631     +9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            621        630     +9   
  Misses            1          1          
  Partials          0          0          

Powered by Codecov. Last update 023007c...5d83d04

@popomore
Copy link
Member Author

Closes eggjs/egg#51

@@ -0,0 +1,10 @@
'use strict';

// TODO can use co
Copy link
Member

Choose a reason for hiding this comment

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

TODO 干掉?

@@ -0,0 +1,11 @@
'use strict';

// TODO can use co
Copy link
Member

Choose a reason for hiding this comment

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

TODO

Copy link
Member Author

Choose a reason for hiding this comment

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

我改下,在飞机上没网络

app.app.should.equal('app');
describe('app.js as function return promise', function() {
let app;
before(function(done) {
Copy link
Member

Choose a reason for hiding this comment

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

() => return app.ready()

@popomore
Copy link
Member Author

改成 co 了

(app.dateC <= app.date).should.equal(true);
describe('app.js as async function', () => {
let app;
before(done => {
Copy link
Member

Choose a reason for hiding this comment

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

before(() => {
  ...
  return app.done();
});

Copy link
Member Author

Choose a reason for hiding this comment

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

这个没关系吧,都一样

@@ -18,19 +20,41 @@ module.exports = {
* doAsync(done);
* }
* ```
*
* it's better to use async function or generator wrapped by co.wrap
Copy link
Member

Choose a reason for hiding this comment

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

co 的也加上示例吧, 毕竟 node 7 LTS 还很遥远

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,我加下

@fengmk2
Copy link
Member

fengmk2 commented Oct 19, 2016

+1


module.exports = co.wrap(function*(app) {
yield wait(1000);
console.log(app);
Copy link
Member

Choose a reason for hiding this comment

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

这个删除了吧

@fengmk2
Copy link
Member

fengmk2 commented Oct 19, 2016

console 删除了吧 @popomore

You can use async function to write async code, never readyCallback.

```js
module.exports = async function(app) {
  await doAsync();
};
```

Or use promise wrap on old node

```js
const co = require('co');
module.exports = co.wrap(function* (app) {
  yield doAsync();
});
```
@popomore
Copy link
Member Author

删掉了

@popomore
Copy link
Member Author

这个差不多可以合了

@popomore
Copy link
Member Author

Windows 先别关,修好了一起改。

@popomore
Copy link
Member Author

@dead-horse 这个有问题么

@dead-horse
Copy link
Member

windows 的测试不用管么?� async function 的没有问题啊,不过还是想把 generator function 的默认支持加上。。

@popomore
Copy link
Member Author

合了吧,网络问题 cnpm/npminstall#152

@popomore
Copy link
Member Author

合完我发个版本

@popomore popomore merged commit 72e8b00 into master Oct 24, 2016
@popomore popomore deleted the app-promise branch October 24, 2016 12:01
@popomore
Copy link
Member Author

@fengmk2 appvoyer 你能 retry?

@popomore
Copy link
Member Author

  • egg-core@0.5.0

fengmk2 added a commit that referenced this pull request Jan 13, 2023
```bash
egg start timeline:
▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [289ms] - #0 Process Start
                  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [510ms] - #1 Application Start
                  ▇ [2ms] - #2 Load Plugin
                  ▇ [1ms] - #3 Load Config
                  ▇ [0ms] - #4 Require(0) ~/eggjs/egg-core/test/fixtures/egg/config/config.default.js
                  ▇ [0ms] - #5 Require(1) ~/eggjs/egg-core/test/fixtures/egg/config/config.unittest.js
                  ▇ [0ms] - #6 Load extend/application.js
                  ▇ [0ms] - #7 Require(2) ~/eggjs/egg-core/test/fixtures/egg/app/extend/application.js
                  ▇ [1ms] - #8 Load extend/context.js
                  ▇ [0ms] - #9 Load extend/request.js
                  ▇ [0ms] - #10 Load extend/response.js
                  ▇ [1ms] - #11 Load app.js
                  ▇ [0ms] - #12 Require(3) ~/eggjs/egg-core/test/fixtures/egg/node_modules/session/app.js
                  ▇ [0ms] - #13 Require(4) app.js
                  ▇▇▇▇▇▇ [101ms] - #14 readyCallback in a
                  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [501ms] - #15 readyCallback in b
                  ▇ [4ms] - #16 Load Middleware
                  ▇ [4ms] - #17 Load "middlewares" to Application
                  ▇ [2ms] - #18 Load Service
                  ▇ [2ms] - #19 Load "service" to Context
                  ▇ [0ms] - #20 Load Controller
                  ▇ [0ms] - #21 Load "controller" to Application
                  ▇ [0ms] - #22 Load Router
```
fengmk2 added a commit that referenced this pull request Jan 13, 2023
```bash
egg start timeline:
▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [289ms] - #0 Process Start
                  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [510ms] - #1 Application Start
                  ▇ [2ms] - #2 Load Plugin
                  ▇ [1ms] - #3 Load Config
                  ▇ [0ms] - #4 Require(0) ~/eggjs/egg-core/test/fixtures/egg/config/config.default.js
                  ▇ [0ms] - #5 Require(1) ~/eggjs/egg-core/test/fixtures/egg/config/config.unittest.js
                  ▇ [0ms] - #6 Load extend/application.js
                  ▇ [0ms] - #7 Require(2) ~/eggjs/egg-core/test/fixtures/egg/app/extend/application.js
                  ▇ [1ms] - #8 Load extend/context.js
                  ▇ [0ms] - #9 Load extend/request.js
                  ▇ [0ms] - #10 Load extend/response.js
                  ▇ [1ms] - #11 Load app.js
                  ▇ [0ms] - #12 Require(3) ~/eggjs/egg-core/test/fixtures/egg/node_modules/session/app.js
                  ▇ [0ms] - #13 Require(4) app.js
                  ▇▇▇▇▇▇ [101ms] - #14 readyCallback in a
                  ▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇▇ [501ms] - #15 readyCallback in b
                  ▇ [4ms] - #16 Load Middleware
                  ▇ [4ms] - #17 Load "middlewares" to Application
                  ▇ [2ms] - #18 Load Service
                  ▇ [2ms] - #19 Load "service" to Context
                  ▇ [0ms] - #20 Load Controller
                  ▇ [0ms] - #21 Load "controller" to Application
                  ▇ [0ms] - #22 Load Router
```
atian25 pushed a commit that referenced this pull request Jan 13, 2023
[skip ci]

## [5.3.0](v5.2.0...v5.3.0) (2023-01-13)

### Features

* support show app.timing in timline string ([#260](#260)) ([5b7af12](5b7af12)), closes [#0](https://github.com/eggjs/egg-core/issues/0) [#1](#1) [#2](#2) [#3](#3) [#4](#4) [#5](#5) [#6](#6) [#7](#7) [#8](#8) [#9](#9) [#10](#10) [#11](#11) [#12](#12) [#13](#13) [#14](#14) [#15](#15) [#16](#16) [#17](#17) [#18](#18) [#19](#19) [#20](#20) [#21](#21) [#22](#22)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

app.js/agent.js 支持返回一个 promise
5 participants