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

[bugfix] 重复绑定底层事件,事件中心增加 one,realtime 暴露 clientId。 #131

Merged
merged 6 commits into from
Jul 13, 2015

Conversation

wangxiao
Copy link
Contributor

@leeyeh
Copy link
Contributor

leeyeh commented Jul 10, 2015

关于事件中心的设计,我仔细想了想,为了减少用户的学习与记忆成本,最好还是与流行的产品接近。比如可以参照 jQuery(onoffone),或者 nodejs 的 EventEmitteron/addListnerremoveListneronce),个人偏好后者(node 中直接用,浏览器也有现成的实现)。

不论是那种,off 都是不用传参数的,内部也不需要维护两个回调数组(否则我 off 的时候还要想想是怎哪种方式注册的)。

然后「事件多次绑定只绑定一次」这个 option 很坑,尤其是与 once 一组合这策略想都想不清楚。我觉得这种问题我们是不是还是通过文档、运行时 warnning 的形式来教育用户比较合适。

@leeyeh
Copy link
Contributor

leeyeh commented Jul 10, 2015

另外,现在有三个 flag(openFlagcloseFlagreuseFlag),flag 之间应该是正交的,这里是不是可以用状态转移来表示更合适。

if (cache.openFlag) {
return;
}

cache.ec.on('session-opened', function(data) {
// 标记重试状态为 false,表示没有在重试
cache.resuseFlag = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: reuseFlag

wangxiao added a commit that referenced this pull request Jul 13, 2015
[bugfix] 重复绑定底层事件,事件中心增加 one,realtime 暴露 clientId。
@wangxiao wangxiao merged commit 5bf0e22 into leancloud:master Jul 13, 2015
@wangxiao
Copy link
Contributor Author

感谢 review。

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.

None yet

2 participants