From ce8f156f51af0a38bc6c9e454786f7687da76d61 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sun, 7 Feb 2021 16:47:56 +0000 Subject: [PATCH] Desktop: Fixed issue which could cause plugin views to be orphaned --- packages/app-desktop/app.ts | 28 ++++++++++++ .../app-desktop/gui/MainScreen/MainScreen.tsx | 44 ++++++++++--------- .../lib/services/plugins/PluginService.ts | 17 +++++++ 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/packages/app-desktop/app.ts b/packages/app-desktop/app.ts index 315c0529d0..a968771277 100644 --- a/packages/app-desktop/app.ts +++ b/packages/app-desktop/app.ts @@ -121,6 +121,7 @@ export interface AppState extends State { visibleDialogs: any; // empty object if no dialog is visible. Otherwise contains the list of visible dialogs. focusedField: string; layoutMoveMode: boolean; + startupPluginsLoaded: boolean; // Extra reducer keys go here watchedResources: any; @@ -144,11 +145,14 @@ const appDefaultState: AppState = { focusedField: null, layoutMoveMode: false, mainLayout: null, + startupPluginsLoaded: false, ...resourceEditWatcherDefaultState, }; class Application extends BaseApplication { + private checkAllPluginStartedIID_: any = null; + constructor() { super(); @@ -200,6 +204,20 @@ class Application extends BaseApplication { } break; + case 'STARTUP_PLUGINS_LOADED': + + // When all startup plugins have loaded, we also recreate the + // main layout to ensure that it is updated in the UI. There's + // probably a cleaner way to do this, but for now that will do. + if (state.startupPluginsLoaded !== action.value) { + newState = { + ...newState, + startupPluginsLoaded: action.value, + mainLayout: JSON.parse(JSON.stringify(newState.mainLayout)), + }; + } + break; + case 'WINDOW_CONTENT_SIZE_SET': newState = Object.assign({}, state); @@ -552,6 +570,16 @@ class Application extends BaseApplication { } catch (error) { this.logger().error(`There was an error loading plugins from ${Setting.value('plugins.devPluginPaths')}:`, error); } + + this.checkAllPluginStartedIID_ = setInterval(() => { + if (service.allPluginsStarted) { + clearInterval(this.checkAllPluginStartedIID_); + this.dispatch({ + type: 'STARTUP_PLUGINS_LOADED', + value: true, + }); + } + }, 500); } async start(argv: string[]): Promise { diff --git a/packages/app-desktop/gui/MainScreen/MainScreen.tsx b/packages/app-desktop/gui/MainScreen/MainScreen.tsx index 836377bc49..9dad2c4a70 100644 --- a/packages/app-desktop/gui/MainScreen/MainScreen.tsx +++ b/packages/app-desktop/gui/MainScreen/MainScreen.tsx @@ -62,6 +62,7 @@ interface Props { themeId: number; settingEditorCodeView: boolean; pluginsLegacy: any; + startupPluginsLoaded: boolean; } interface State { @@ -616,19 +617,21 @@ class MainScreenComponent extends React.Component { if (components[key]) return components[key](); + const viewsToRemove: string[] = []; + if (key.indexOf('plugin-view') === 0) { const viewInfo = pluginUtils.viewInfoByViewId(this.props.plugins, event.item.key); if (!viewInfo) { - // Note that it will happen when the component is rendered - // before the plugins have loaded their views, so because of - // this we need to keep the view in the layout. + // Once all startup plugins have loaded, we know that all the + // views are ready so we can remove the orphans ones. // - // But it can also be a problem if the view really is invalid - // due to a faulty plugin as currently there would be no way to - // remove it. - console.warn(`Could not find plugin associated with view: ${event.item.key}`); - return null; + // Before they are loaded, there might be views that don't match + // any plugins, but that's only because it hasn't loaded yet. + if (this.props.startupPluginsLoaded) { + console.warn(`Could not find plugin associated with view: ${event.item.key}`); + viewsToRemove.push(event.item.key); + } } else { const { view, plugin } = viewInfo; @@ -647,19 +650,19 @@ class MainScreenComponent extends React.Component { throw new Error(`Invalid layout component: ${key}`); } - // if (viewsToRemove.length) { - // window.requestAnimationFrame(() => { - // let newLayout = this.props.mainLayout; - // for (const itemKey of viewsToRemove) { - // newLayout = removeItem(newLayout, itemKey); - // } + if (viewsToRemove.length) { + window.requestAnimationFrame(() => { + let newLayout = this.props.mainLayout; + for (const itemKey of viewsToRemove) { + newLayout = removeItem(newLayout, itemKey); + } - // if (newLayout !== this.props.mainLayout) { - // console.warn('Removed invalid views:', viewsToRemove); - // this.updateMainLayout(newLayout); - // } - // }); - // } + if (newLayout !== this.props.mainLayout) { + console.warn('Removed invalid views:', viewsToRemove); + this.updateMainLayout(newLayout); + } + }); + } } renderPluginDialogs() { @@ -773,6 +776,7 @@ const mapStateToProps = (state: AppState) => { focusedField: state.focusedField, layoutMoveMode: state.layoutMoveMode, mainLayout: state.mainLayout, + startupPluginsLoaded: state.startupPluginsLoaded, }; }; diff --git a/packages/lib/services/plugins/PluginService.ts b/packages/lib/services/plugins/PluginService.ts index 8c9bc1b48a..240ba8954d 100644 --- a/packages/lib/services/plugins/PluginService.ts +++ b/packages/lib/services/plugins/PluginService.ts @@ -73,6 +73,7 @@ export default class PluginService extends BaseService { private platformImplementation_: any = null; private plugins_: Plugins = {}; private runner_: BasePluginRunner = null; + private startedPlugins_: Record = {}; public initialize(appVersion: string, platformImplementation: any, runner: BasePluginRunner, store: any) { this.appVersion_ = appVersion; @@ -337,6 +338,13 @@ export default class PluginService extends BaseService { return compareVersions(this.appVersion_, pluginVersion) >= 0; } + public get allPluginsStarted(): boolean { + for (const pluginId of Object.keys(this.startedPlugins_)) { + if (!this.startedPlugins_[pluginId]) return false; + } + return true; + } + public async runPlugin(plugin: Plugin) { if (!this.isCompatible(plugin.manifest.app_min_version)) { throw new Error(`Plugin "${plugin.id}" was disabled because it requires Joplin version ${plugin.manifest.app_min_version} and current version is ${this.appVersion_}.`); @@ -351,6 +359,15 @@ export default class PluginService extends BaseService { }); } + this.startedPlugins_[plugin.id] = false; + + const onStarted = () => { + this.startedPlugins_[plugin.id] = true; + plugin.off('started', onStarted); + }; + + plugin.on('started', onStarted); + const pluginApi = new Global(this.platformImplementation_, plugin, this.store_); return this.runner_.run(plugin, pluginApi); }