refactor(Use vue 3 + TypeScript to reconstruct the original plugin): … #127
Conversation
…Delete the obsolete vue 2 method in the code and replace it with the wording supported by vue 3 重构插件让其支持了vue3，未修改代码中的单元测试部分代码。
|
Is rewrite to typescript planned? What need to be done to merge this pull request? |
|
yes，Already done。 |
|
Do you have permission to merge? |
|
good big white @likaia |
|
@likaia good work, but it would be useful to have comments in English to make it clears to everyone |
|
Yes, the translation plan is on the agenda. |
| "description": "native websocket implemantation for vuejs and vuex", | ||
| "main": "dist/build.js", | ||
| "main": "dist/Main.js", |
There was a problem hiding this comment.
Capitalization is fine, Because the main file is Main.ts, I capitalized.
| removeListener(label: T, callback: () => void, vm: T): boolean { | ||
| // 从监听列表中获取当前事件 | ||
| const listeners = this.listeners.get(label); | ||
| let index; |
There was a problem hiding this comment.
right, should be index: number yes?
|
|
||
| if (listeners && listeners.length) { | ||
| // 寻找当前事件在事件监听列表的位置 | ||
| index = listeners.reduce((i: any, listener: any, index: T) => { |
There was a problem hiding this comment.
i and index should be type number
| @@ -0,0 +1,108 @@ | |||
| import Observer from "./Observer"; | |||
There was a problem hiding this comment.
this file really should be main.ts since it's not a Class file, and is consistent with other library naming conventions
There was a problem hiding this comment.
No, how can Observer replace the entry file of the main.ts plugin?
There was a problem hiding this comment.
sorry, there's no way to comment on a file itself. I was just saying that this file Main.ts should be renamed to main.ts and then update accordingly in the package.json, per my other comment
| @@ -0,0 +1,28 @@ | |||
| // 插件内用到的类型进行统一定义 | |||
There was a problem hiding this comment.
can this file be renamed to just types.ts ?
There was a problem hiding this comment.
I wanted to change it to type.ts, but I think my current name looks a little more intuitive
There was a problem hiding this comment.
perhaps lib-types.ts would be more consistent?
| */ | ||
| defaultPassToStore( | ||
| eventName: string, | ||
| event: { |
There was a problem hiding this comment.
how about VuexStore since that's really what this property is defining
|
English documentation and English comments have been added. link: https://github.com/likaia/vue-native-websocket-vue3/blob/master/README-EN.md |
| } | ||
|
|
||
| /** | ||
| * 添加事件监听 |
There was a problem hiding this comment.
These doc comments need to at least have English translations.
…Delete the obsolete vue 2 method in the code and replace it with the wording supported by vue 3
重构插件让其支持了vue3，未修改代码中的单元测试部分代码。