feat(EventEmitter): 新增事件链式注册，统一清除功能 #1968
Conversation
|
|
@ChangeSuger 👋，这里是事件系统的调整代码 |
|
嗨~ 你能补充一下这个功能的使用场景吗？然后最好在 feature-example 项目中增加一个使用 demo。 我希望能解释一下增加这个功能的必要性 因为现在在我看来，我们支持一次注册多个时间 eventCenter.on('x1,x2,x3', () => { // callback } )；也应该支持一次性 eventCenter.off('x1,x2,x3', () => {}) 这样。链式调用的好处和必要性是什么呢❓（因为我目前看注释，定义 off 方法，是链式调用的返回结果，后面调用 off 的地方，可能还不如我上面这个写法直观，需要回溯去看 off 的定义，再看是否能直接复用） |
|
我刚在路上突然想到，你这个设计好像跟我们之前的用法也不冲突，好像也没有啥问题 |
我的想法是这样的，对于多个不同的事件，应该更友好的支持用户使用不同的回调函数。并且修改之后的代码也兼容以前的写法 |
packages/core/src/LogicFlow.tsx
Outdated
| on<T extends string>(evt: T, callback: EventCallback<T>): void | ||
| on(evt: string, callback: EventCallback) { | ||
| this.graphModel.eventCenter.on(evt, callback) | ||
| on: OnMethod = (evt: string, callback: EventCallback, once?: boolean) => { |
There was a problem hiding this comment.
我感觉类型命名 OnEvent 会比 OnMethod 好一些
There was a problem hiding this comment.
确实是这样 OnEvent 是要更好一点
packages/core/src/LogicFlow.tsx
Outdated
| once<T extends string>(evt: T, callback: EventCallback<T>): void | ||
| once(evt: string, callback: EventCallback) { | ||
| this.graphModel.eventCenter.once(evt, callback) | ||
| once: OnceMethod = (evt: string, callback: EventCallback) => { |
There was a problem hiding this comment.
同上，OnceMethod -> OnceEvent
| callback: EventCallback<T>, | ||
| once?: boolean, | ||
| ): ClearMethod | ||
| (evt: string, callback: EventCallback, once?: boolean): ClearMethod |
There was a problem hiding this comment.
类型定义这儿不需要 35 这一行的类型定义了感觉，因为这一行会导致使用时 ts 类型提示失效
类型重载，上面两个是类型，下面是方法定义。
现在这种写法会导致 上图上中类型提示失效，你可以再确认下
以下为参考：
type OnEvent = {
<T extends keyof EventArgs>(evt: T, callback: EventCallback<T>): void;
<T extends string>(evt: T, callback: EventCallback<T>): void;
};
once: OnceEvent = (evt: string, callback: EventCallback) => {
this.graphModel.eventCenter.once(evt, callback);
};
There was a problem hiding this comment.
我是了一下，并不会导致类型失效。但其实按照我们现有的代码逻辑，永远不会走到最后一个函数重载，删掉也是对的
There was a problem hiding this comment.
我是了一下，并不会导致类型失效。但其实按照我们现有的代码逻辑，永远不会走到最后一个函数重载，删掉也是对的
@admin1949 补充一下类型提示的区别你看下
当前 PR：
之前的类型提示：
你看下提示的区别。 @ChangeSuger 你有空可以看下这个问题怎么解？这个 PR 别的应该都 OK，就现在这个有点问题，哈哈哈
There was a problem hiding this comment.
@boyongjiong 我将代码切换到最新的master分支测试了一下出来的效果和我提交PR之后是一样的，我认为这只是编辑器对于类型提示的风格不一样，并不会影响实际的功能
There was a problem hiding this comment.
@boyongjiong 我将代码切换到最新的master分支测试了一下出来的效果和我提交PR之后是一样的，我认为这只是编辑器对于类型提示的风格不一样，并不会影响实际的功能
这个是 vscode 吗？你鼠标放到 data 上会出现 data 的类型提示吗？像下面这样
我纠结的点是因为如果提示的是 NodeEventArgsPick<> 这个，对用户是无帮助的，像下面这样才是符合预期的
There was a problem hiding this comment.
有的呀，我用 vscode 和 WebStorm 都检验过了
这是vscode的截图
这里是 WebStorm 的截图
| (evt: string, callback: EventCallback): ClearMethod | ||
| } | ||
|
|
||
| interface ClearMethod { |
There was a problem hiding this comment.
建议：ClearMethod -> ClearCallback
| evt: T, | ||
| callback: EventCallback<T>, | ||
| on: OnMethod = ( | ||
| evt: string, |
There was a problem hiding this comment.
这块儿有点问题，evt 不能直接写成 string，上面说的类型提示失效好像是这块儿的问题，得再确认一下
| @@ -149,4 +198,29 @@ export default class EventEmitter { | |||
| } | |||
| } | |||
|
|
|||
| const createClearMethod = ( | |||
OnMethod -> OnEvent 删除无用的类型注释
|
对了，我还忘了一件事儿，咱这个功能，应该在文档里面也体现一下，你看下官网项目，找到对应的事件，加一块儿进去写一下？ |
嗯嗯，我去弄下 |
增强现有的事件系统的注册，清除功能