Mobile: Fix plugins aren't visible after switching to a new profile (#…

…10386)

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<AppState> = null;
+
 const shouldShowBasedOnSettingSearchQuery = ()=>true;
 const PluginStatesWrapper = (props: WrapperProps) => {
 	const styles = configScreenStyles(Setting.THEME_LIGHT);
@@ -34,8 +39,8 @@ const PluginStatesWrapper = (props: WrapperProps) => {
 
 	return (
 		<PluginStates
-			themeId={Setting.THEME_LIGHT}
 			styles={styles}
+			themeId={Setting.THEME_LIGHT}
 			updatePluginStates={updatePluginStates}
 			pluginSettings={pluginSettings}
 			shouldShowBasedOnSearchQuery={shouldShowBasedOnSettingSearchQuery}
@@ -65,6 +70,7 @@ const loadMockPlugin = async (id: string, name: string, version: string, pluginS
 		app_min_version: '1.4',
 		name,
 		description: 'Test plugin',
+		platforms: ['mobile', 'desktop'],
 		version,
 		homepage_url: 'https://joplinapp.org',
 	})}
@@ -76,26 +82,27 @@ const loadMockPlugin = async (id: string, name: string, version: string, pluginS
 	`;
 	const pluginPath = join(await createTempDir(), 'plugin.js');
 	await writeFile(pluginPath, pluginSource, 'utf-8');
-	await service.loadAndRunPlugins([pluginPath], pluginSettings);
+	await act(async () => {
+		await service.loadAndRunPlugins([pluginPath], pluginSettings);
+	});
 };
 
 describe('PluginStates', () => {
 	beforeEach(async () => {
 		await setupDatabaseAndSynchronizer(0);
 		await switchClient(0);
-		pluginServiceSetup();
+		reduxStore = createMockReduxStore();
+		pluginServiceSetup(reduxStore);
 		resetRepoApi();
 
 		await mockMobilePlatform('android');
 		await mockRepositoryApiConstructor();
 	});
 	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(
+			<PluginStatesWrapper
+				initialPluginSettings={pluginSettings}
+			/>,
+		);
+
+		// 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();
+	});
 });

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 [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})`);
-		}
+const useLoadedPluginIds = () => {
+	const getLoadedPlugins = useCallback(() => {
+		return PluginService.instance().pluginIds;
+	}, []);
+	const [loadedPluginIds, setLoadedPluginIds] = useState(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);
-	}
+	useAsyncEffect(async event => {
+		while (!event.cancelled) {
+			await PluginService.instance().waitForLoadedPluginsChange();
+			setLoadedPluginIds(getLoadedPlugins());
+		}
+	}, []);
 
 	return loadedPluginIds;
 };
@@ -130,7 +110,7 @@ const PluginStates: React.FC<Props> = 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];
 

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);

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,
 	);
 };
 

packages/app-mobile/plugins/PluginRunner/PluginRunnerWebView.tsx

@@ -95,8 +95,9 @@ const PluginRunnerWebViewComponent: React.FC<Props> = 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);

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);
 			}

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<string, boolean> = {};
 	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,18 +142,34 @@ export default class PluginService extends BaseService {
 		this.isSafeMode_ = v;
 	}
 
+	public waitForLoadedPluginsChange() {
+		return new Promise<void>(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) {
 		if (!this.plugins_[pluginId]) return;
 
 		this.plugins_ = { ...this.plugins_ };
 		delete this.plugins_[pluginId];
+
+		this.dispatchPluginsChangeListeners();
 	}
 
 	public async unloadPlugin(pluginId: string) {