-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add onDataChanged method to handle data changes in the plugin #16244
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
base: dev
Are you sure you want to change the base?
Conversation
c2ba4dc to
f387e77
Compare
|
可以 review 了 |
|
|
||
| export const reloadPlugin = async (app: App, data: { upsertPlugins: string[], removePlugins: string[] }) => { | ||
| data.removePlugins.forEach((item) => { | ||
| export const reloadPlugin = async (app: App, data: { upsertCodePlugins?: string[], upsertDataPlugins?: string[], removePlugins?: string[] } = {}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upsertCodePlugins 这个为什么要加 Code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
插件代码有变化就需要重新加载插件
|
|
||
| export const reloadPlugin = async (app: App, data: { upsertPlugins: string[], removePlugins: string[] }) => { | ||
| data.removePlugins.forEach((item) => { | ||
| export const reloadPlugin = async (app: App, data: { upsertCodePlugins?: string[], upsertDataPlugins?: string[], removePlugins?: string[] } = {}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upsertDataPlugins 会有对应的 removeData 么?这个参数是 Data,应该不用加 plugins 后缀了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
插件删除配置文件也算在 upsertDataPlugins 内
| } | ||
| }); | ||
| } | ||
| reloadPlugins.push(pluginName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要去重
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
看了你的回复,发现这并不是处理 data change 的。抱歉,没注意看前面的 #15444 (comment) 。如果要处理禁用状态下卸载插件的话,可以加一个阶段为 uninstalled,即卸载成功后的回调。你看这样可以么? |
|
前面那个 issue 之所以需要在没有启用插件的情况下卸载插件,是因为把 onunload 方法用来处理插件配置同步了,现在增加一个新的 onDataChanged 方法就可以使用正常的 onunload 方法在前端卸载插件了。 我感觉没有什么办法能在不启用插件的情况下执行 uninstall。 |
|
这个是卸载的问题,和 data change 没有关系。这样开发者可能无法理解,我也不太理解。一开始我就误解为插接数据变化的同步。 |
|
确实是插件存储的数据的变化,就是 this.data |
|
正常情况下 onunload() 应该移除插件在前端添加的所有组件,但之前的逻辑是存储数据变更会先执行 onunload() 再执行 onload(),同步一下插件配置就会把组件全部移除的话交互太奇怪了,所以我就把完整的 onunload() 逻辑移到 uninstall() 里去了,然后就会导致一个问题,如果用户先禁用插件再卸载插件的话就只会执行 onunload(),一部分组件还留在界面上。 增加一个单独的 onDataChanged() 方法的话,我就可以把 uninstall() 里的逻辑移回 onunload(),把 onDataChanged() 用来处理配置同步,onunload() 用来完整移除组件,uninstall() 只额外执行一步 removeData(),这样就基本没有在禁用插件时卸载插件执行 uninstall() 的需求了。 |
|
实际上这个 PR 单纯就是用来处理数据同步的,不看之前那个 issue 也完全没有影响 |
|
如果不看前一个 issue,把这个做为数据更改的话,它又和 onunload,uninstall 这些生命周期混杂在一起。还是没搞懂这个的用途和价值。 |
|
目前数据同步之后插件会执行 onunload() 和 onload(),增加一个 onDataChanged() 就不需要执行 onunload() 和 onload() 了 |
|
但是 changeDate 不应该和 onunload() onload() 搅合在一起 |
|
我没理解你的意思 |
给插件添加 onDataChanged 方法,用来替代同步之后插件先 onunload 再 onload,解决我在 #15444 (comment) 遇到的问题
基于前一个 PR 修改,需要先合并:#16243
需要测试: