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: listener add async function support #13

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat: listener add async function support #13

wants to merge 4 commits into from

Conversation

kurten
Copy link

@kurten kurten commented Apr 4, 2018

rt
listener support async function

index.js Outdated
const ret = yield listener(...args);
if (is.promise(ret)) {
yield ret;
}
Copy link
Member

Choose a reason for hiding this comment

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

这里要 generator 和 async 分开判断,async 不要包 co 了

Copy link
Author

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.

没办法,包一层 async 的化就只处理下 catch 就好了

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "sdk-base",
"version": "3.4.0",
"version": "3.4.1",
Copy link
Member

Choose a reason for hiding this comment

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

由发布的人来改版本,这里应该发 minor

Copy link
Author

Choose a reason for hiding this comment

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

这里可以改进

@gxcsoccer
Copy link
Member

另外 ci 要搞过

@gxcsoccer
Copy link
Member

async 的用例在 node 6 下跑不了

@kurten
Copy link
Author

kurten commented Apr 4, 2018

依赖了babel,能过node 6测试

test/.setup.js Outdated
@@ -0,0 +1,3 @@
'use strict';

require('babel-register');
Copy link
Member

Choose a reason for hiding this comment

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

这里要特性探测下,node 6 才引入 babel

yield listener(...args);
});
}
promise.catch(err => {
Copy link
Member

Choose a reason for hiding this comment

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

removeListener 的时候也要支持下 async function

@kurten
Copy link
Author

kurten commented Apr 5, 2018

已经修复

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.

2 participants