Use supplied path when installing plugin from NPM to prevent escaping issues (#3825)
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()`](69bac4a3d6/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)
<img width="1541" alt="Screen Shot 2022-06-20 at 1 18 21 pm" src="https://user-images.githubusercontent.com/33126/174590735-ed25d192-a661-4333-af08-e494678b9fbb.png">
- logs will show a failure to read `package.json`
<img width="1175" alt="Screen Shot 2022-06-20 at 1 18 39 pm" src="https://user-images.githubusercontent.com/33126/174590824-339ba7ef-dcde-42b6-90ac-b99424845c3e.png">
### 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
<img width="1541" alt="image" src="https://user-images.githubusercontent.com/33126/174591112-880f55bf-0439-457c-a199-4bab0b3f193f.png">
- (package can also be successfully used/upgraded/uninstalled)
Reviewed By: lawrencelomax
Differential Revision: D39345564
Pulled By: lblasa
fbshipit-source-id: 729d70a29c7941e59ac03bb21061fc1d2bc8d998
This commit is contained in:
committed by
Facebook GitHub Bot
parent
6fcaaabb17
commit
13afb8b802
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user