From 13afb8b8024db49303315cb63daf39aa95ce6662 Mon Sep 17 00:00:00 2001 From: Joshua May Date: Thu, 8 Sep 2022 04:27:00 -0700 Subject: [PATCH] Use supplied path when installing plugin from NPM to prevent escaping issues (#3825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Resolves https://github.com/facebook/flipper/issues/3819 This PR fixes Flipper's plugin installation, so that scoped plugins (with `/` in the name) can be successfully installed (+removed). In more detail, Flipper's function [`installPluginFromNpm()`](https://github.com/facebook/flipper/blob/69bac4a3d6b5546ef6bd05b8124c33304380a758/desktop/plugin-lib/src/pluginInstaller.tsx#L99-L114) relies on the `live-plugin-manager` package to handle the NPM package to a temporary directory. Before this patch, Flipper would assume the name of the directory (incorrectly). This patch simply uses the directory as provided by `live-plugin-manager`'s result. If we use `shopify/flipper-plugin-react-native-performance` as a concrete example after this patch: - `installPluginFromNpm()` is called to install `shopify/flipper-plugin-react-native-performance` - `plugManNoDep.install()` installs the plugin to `/path/to/tmp/shopify/flipper-plugin-react-native-performance` - note: before this patch, it would have been assumed to be installed to: `/path/to/tmp/shopify__flipper-plugin-react-native-performance` which does not exist - `installPluginFromTempDir()` is called to install the package to `~/.flipper/installed-plugins/shopify__flipper-plugin-react-native-performance/{version}` (where `{version}` is the current version, i.e. `1.0.0`) Once the plugin is in `~/.flipper/installed-plugins/`, the escaping is correctly applied consistently by Flipper, and there are no further issues managing/using the plugin. ## Changelog Fixed errors when installing scoped NPM plugins Pull Request resolved: https://github.com/facebook/flipper/pull/3825 Test Plan: Ideally we'd have some unit tests, but mocking out the NPM fetching in `live-plugin-manager` seems like a bunch of work. If you have some shortcuts, let me know, because this would be useful to test across platforms. But in the meantime, we can easily manually test this. ### Reproduce the failure First, let's reproduce the failure: - run Flipper 0.150.0 (or `yarn start` on master, etc) - open plugin marketplace, attempt to install scoped plugins - installation will fail (with ⚠️ icon) Screen Shot 2022-06-20 at 1 18 21 pm - logs will show a failure to read `package.json` Screen Shot 2022-06-20 at 1 18 39 pm ### Demonstrate success - run Flipper from this branch (via `yarn start`) - open plugin marketplace, attempt to install scoped plugins - installation will succeed, with notification to restart Flipper image - (package can also be successfully used/upgraded/uninstalled) Reviewed By: lawrencelomax Differential Revision: D39345564 Pulled By: lblasa fbshipit-source-id: 729d70a29c7941e59ac03bb21061fc1d2bc8d998 --- desktop/plugin-lib/src/pluginInstaller.tsx | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/desktop/plugin-lib/src/pluginInstaller.tsx b/desktop/plugin-lib/src/pluginInstaller.tsx index 6cc04ae9e..eaa0bf16f 100644 --- a/desktop/plugin-lib/src/pluginInstaller.tsx +++ b/desktop/plugin-lib/src/pluginInstaller.tsx @@ -22,7 +22,6 @@ import {InstalledPluginDetails} from 'flipper-common'; import {getInstalledPluginDetails, isPluginDir} from './getPluginDetails'; import { getPluginVersionInstallationDir, - getPluginDirNameFromPackageName, getPluginInstallationDir, pluginInstallationDir, legacyPluginInstallationDir, @@ -102,12 +101,8 @@ export async function installPluginFromNpm(name: string) { await fs.ensureDir(tmpDir); const plugManNoDep = providePluginManagerNoDependencies(); plugManNoDep.options.pluginsPath = tmpDir; - await plugManNoDep.install(name); - const pluginTempDir = path.join( - tmpDir, - getPluginDirNameFromPackageName(name), - ); - return await installPluginFromTempDir(pluginTempDir); + const pluginInfo = await plugManNoDep.install(name); + return await installPluginFromTempDir(pluginInfo.location); } finally { await fs.remove(tmpDir); }