diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx index e81f26e00..2192efd2a 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.test.tsx @@ -1,8 +1,8 @@ import * as React from 'react'; import RepositoryApi from '@joplin/lib/services/plugins/RepositoryApi'; -import { afterAllCleanUp, afterEachCleanUp, createTempDir, mockMobilePlatform, setupDatabaseAndSynchronizer, supportDir, switchClient } from '@joplin/lib/testing/test-utils'; +import { createTempDir, mockMobilePlatform, setupDatabaseAndSynchronizer, supportDir, switchClient } from '@joplin/lib/testing/test-utils'; -import { render, screen } from '@testing-library/react-native'; +import { act, render, screen } from '@testing-library/react-native'; import '@testing-library/react-native/extend-expect'; import Setting from '@joplin/lib/models/Setting'; @@ -15,11 +15,16 @@ import { remove, writeFile } from 'fs-extra'; import { join } from 'path'; import shim from '@joplin/lib/shim'; import { resetRepoApi } from './utils/useRepoApi'; +import { Store } from 'redux'; +import { AppState } from '../../../../utils/types'; +import createMockReduxStore from '../../../../utils/testing/createMockReduxStore'; interface WrapperProps { initialPluginSettings: PluginSettings; } +let reduxStore: Store = null; + const shouldShowBasedOnSettingSearchQuery = ()=>true; const PluginStatesWrapper = (props: WrapperProps) => { const styles = configScreenStyles(Setting.THEME_LIGHT); @@ -34,8 +39,8 @@ const PluginStatesWrapper = (props: WrapperProps) => { return ( { + await service.loadAndRunPlugins([pluginPath], pluginSettings); + }); }; describe('PluginStates', () => { beforeEach(async () => { await setupDatabaseAndSynchronizer(0); await switchClient(0); - pluginServiceSetup(); + reduxStore = createMockReduxStore(); + pluginServiceSetup(reduxStore); resetRepoApi(); await mockMobilePlatform('android'); @@ -91,11 +100,9 @@ describe('PluginStates', () => { }); afterEach(async () => { for (const pluginId of PluginService.instance().pluginIds) { - await PluginService.instance().unloadPlugin(pluginId); + await act(() => PluginService.instance().unloadPlugin(pluginId)); } - await afterEachCleanUp(); }); - afterAll(() => afterAllCleanUp()); it.each([ 'android', @@ -157,4 +164,32 @@ describe('PluginStates', () => { expect(await screen.findByRole('button', { name: 'Update ABC Sheet Music', disabled: false })).toBeVisible(); expect(await screen.findByText(`v${outdatedVersion}`)).toBeVisible(); }); + + it('should update the list of installed plugins when a plugin is installed and uninstalled', async () => { + const pluginSettings: PluginSettings = { }; + + render( + , + ); + + // Initially, no plugins should be visible. + expect(screen.queryByText(/^ABC Sheet Music/)).toBeNull(); + + const testPluginId1 = 'org.joplinapp.plugins.AbcSheetMusic'; + const testPluginId2 = 'org.joplinapp.plugins.test.plugin.id'; + await act(() => loadMockPlugin(testPluginId1, 'ABC Sheet Music', '1.2.3', pluginSettings)); + await act(() => loadMockPlugin(testPluginId2, 'A test plugin', '1.0.0', pluginSettings)); + expect(PluginService.instance().plugins[testPluginId1]).toBeTruthy(); + + // Should update the list of installed plugins even though the plugin settings didn't change. + expect(await screen.findByText(/^ABC Sheet Music/)).toBeVisible(); + expect(await screen.findByText(/^A test plugin/)).toBeVisible(); + + // Uninstalling one plugin should keep the other in the list + await act(() => PluginService.instance().uninstallPlugin(testPluginId1)); + expect(await screen.findByText(/^A test plugin/)).toBeVisible(); + expect(screen.queryByText(/^ABC Sheet Music/)).toBeNull(); + }); }); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx index 8af0d2d38..58a66fef3 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.tsx @@ -1,5 +1,5 @@ import * as React from 'react'; -import { useCallback, useMemo, useRef, useState } from 'react'; +import { useCallback, useState } from 'react'; import { ConfigScreenStyles } from '../configScreenStyles'; import { View } from 'react-native'; import { Banner, Button, Text } from 'react-native-paper'; @@ -11,8 +11,7 @@ import { ItemEvent } from '@joplin/lib/components/shared/config/plugins/types'; import NavService from '@joplin/lib/services/NavService'; import useRepoApi from './utils/useRepoApi'; import RepositoryApi from '@joplin/lib/services/plugins/RepositoryApi'; -import shim from '@joplin/lib/shim'; -import Logger from '@joplin/utils/Logger'; +import useAsyncEffect from '@joplin/lib/hooks/useAsyncEffect'; interface Props { themeId: number; @@ -37,38 +36,19 @@ export const getSearchText = () => { return searchText; }; -const logger = Logger.create('PluginStates'); - // Loaded plugins: All plugins with available manifests. -const useLoadedPluginIds = (pluginSettings: SerializedPluginSettings) => { - const allPluginIds = useMemo(() => { - return Object.keys( - PluginService.instance().unserializePluginSettings(pluginSettings), - ); - }, [pluginSettings]); +const useLoadedPluginIds = () => { + const getLoadedPlugins = useCallback(() => { + return PluginService.instance().pluginIds; + }, []); + const [loadedPluginIds, setLoadedPluginIds] = useState(getLoadedPlugins); - const [pluginReloadCounter, setPluginReloadCounter] = useState(0); - const loadedPluginIds = useMemo(() => { - if (pluginReloadCounter > 0) { - logger.debug(`Not all plugins were loaded in the last render. Re-loading (try ${pluginReloadCounter})`); + useAsyncEffect(async event => { + while (!event.cancelled) { + await PluginService.instance().waitForLoadedPluginsChange(); + setLoadedPluginIds(getLoadedPlugins()); } - - const pluginService = PluginService.instance(); - return allPluginIds.filter(id => !!pluginService.plugins[id]); - }, [allPluginIds, pluginReloadCounter]); - const hasLoadingPlugins = loadedPluginIds.length !== allPluginIds.length; - - // Force a re-render if not all plugins have available metadata. This can happen - // if plugins are still loading. - const pluginReloadCounterRef = useRef(0); - pluginReloadCounterRef.current = pluginReloadCounter; - const timeoutRef = useRef(null); - if (hasLoadingPlugins && !timeoutRef.current) { - timeoutRef.current = shim.setTimeout(() => { - timeoutRef.current = null; - setPluginReloadCounter(pluginReloadCounterRef.current + 1); - }, 1000); - } + }, []); return loadedPluginIds; }; @@ -130,7 +110,7 @@ const PluginStates: React.FC = props => { const installedPluginCards = []; const pluginService = PluginService.instance(); - const pluginIds = useLoadedPluginIds(props.pluginSettings); + const pluginIds = useLoadedPluginIds(); for (const pluginId of pluginIds) { const plugin = pluginService.plugins[pluginId]; diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx index 451d3999e..1160c5087 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/SearchPlugins.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import RepositoryApi, { InstallMode } from '@joplin/lib/services/plugins/RepositoryApi'; -import { afterAllCleanUp, afterEachCleanUp, mockMobilePlatform, setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils'; +import { mockMobilePlatform, setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils'; import { render, screen, userEvent, waitFor } from '@testing-library/react-native'; import '@testing-library/react-native/extend-expect'; @@ -10,6 +10,7 @@ import Setting from '@joplin/lib/models/Setting'; import { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; import pluginServiceSetup from './testUtils/pluginServiceSetup'; import newRepoApi from './testUtils/newRepoApi'; +import createMockReduxStore from '../../../../utils/testing/createMockReduxStore'; interface WrapperProps { repoApi: RepositoryApi; @@ -42,10 +43,8 @@ describe('SearchPlugins', () => { beforeEach(async () => { await setupDatabaseAndSynchronizer(0); await switchClient(0); - pluginServiceSetup(); + pluginServiceSetup(createMockReduxStore()); }); - afterEach(() => afterEachCleanUp()); - afterAll(() => afterAllCleanUp()); it('should find results', async () => { const repoApi = await newRepoApi(InstallMode.Default); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/testUtils/pluginServiceSetup.ts b/packages/app-mobile/components/screens/ConfigScreen/plugins/testUtils/pluginServiceSetup.ts index bbb5a3771..4a9fba72c 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/testUtils/pluginServiceSetup.ts +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/testUtils/pluginServiceSetup.ts @@ -1,10 +1,16 @@ import PluginService from '@joplin/lib/services/plugins/PluginService'; +import { Store } from 'redux'; +import BasePluginRunner from '@joplin/lib/services/plugins/BasePluginRunner'; -const pluginServiceSetup = () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const runner = { run: ()=> {}, stop: ()=>{} } as any; +class MockPluginRunner extends BasePluginRunner { + public override async run() {} + public override async stop() {} +} + +const pluginServiceSetup = (store: Store) => { + const runner = new MockPluginRunner(); PluginService.instance().initialize( - '2.14.0', { joplin: {} }, runner, { dispatch: ()=>{}, getState: ()=>{} }, + '2.14.0', { joplin: {} }, runner, store, ); }; diff --git a/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx b/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx index 916a992ce..48f79ae7e 100644 --- a/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx +++ b/packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx @@ -95,8 +95,9 @@ const PluginRunnerWebViewComponent: React.FC = props => { // To avoid increasing startup time/memory usage on devices with no plugins, don't // load the webview if unnecessary. - // Note that we intentionally load the webview even if all plugins are disabled. - const loadWebView = Object.values(pluginSettings).length > 0 && props.pluginSupportEnabled; + // Note that we intentionally load the webview even if all plugins are disabled, as + // this allows any plugins we don't have settings for to run. + const loadWebView = props.pluginSupportEnabled; useEffect(() => { if (!loadWebView) { setLoaded(false); diff --git a/packages/app-mobile/plugins/loadPlugins.ts b/packages/app-mobile/plugins/loadPlugins.ts index 34aaa18a3..62525fee3 100644 --- a/packages/app-mobile/plugins/loadPlugins.ts +++ b/packages/app-mobile/plugins/loadPlugins.ts @@ -24,7 +24,7 @@ const loadPlugins = async ( pluginService.isSafeMode = Setting.value('isSafeMode'); for (const pluginId of Object.keys(pluginService.plugins)) { - if (!pluginSettings[pluginId]?.enabled) { + if (pluginSettings[pluginId] && !pluginSettings[pluginId].enabled) { logger.info('Unloading disabled plugin', pluginId); await pluginService.unloadPlugin(pluginId); } diff --git a/packages/lib/services/plugins/PluginService.ts b/packages/lib/services/plugins/PluginService.ts index a6ef8b48d..636c35607 100644 --- a/packages/lib/services/plugins/PluginService.ts +++ b/packages/lib/services/plugins/PluginService.ts @@ -80,6 +80,8 @@ function makePluginId(source: string): string { return uslug(source).substr(0, 32); } +type LoadedPluginsChangeListener = ()=> void; + export default class PluginService extends BaseService { private static instance_: PluginService = null; @@ -101,6 +103,7 @@ export default class PluginService extends BaseService { private runner_: BasePluginRunner = null; private startedPlugins_: Record = {}; private isSafeMode_ = false; + private pluginsChangeListeners_: LoadedPluginsChangeListener[] = []; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied public initialize(appVersion: string, platformImplementation: any, runner: BasePluginRunner, store: any) { @@ -139,11 +142,25 @@ export default class PluginService extends BaseService { this.isSafeMode_ = v; } + public waitForLoadedPluginsChange() { + return new Promise(resolve => { + this.pluginsChangeListeners_.push(() => resolve()); + }); + } + + private dispatchPluginsChangeListeners() { + for (const listener of this.pluginsChangeListeners_) { + listener(); + } + this.pluginsChangeListeners_ = []; + } + private setPluginAt(pluginId: string, plugin: Plugin) { this.plugins_ = { ...this.plugins_, [pluginId]: plugin, }; + this.dispatchPluginsChangeListeners(); } private deletePluginAt(pluginId: string) { @@ -151,6 +168,8 @@ export default class PluginService extends BaseService { this.plugins_ = { ...this.plugins_ }; delete this.plugins_[pluginId]; + + this.dispatchPluginsChangeListeners(); } public async unloadPlugin(pluginId: string) {