From 3ff2d3bf99d65615add7464e6f4e62520c6ef0d5 Mon Sep 17 00:00:00 2001 From: Anton Nikolaev Date: Mon, 20 Apr 2020 11:10:27 -0700 Subject: [PATCH] Add "id" field to Flipper plugin manifest Summary: Added "id" field to Flipper plugin manifest which is used to match native and desktop plugin parts. Before that, "name" field was used both as npm package name and as plugin id. The problem is that currently there are a lot of plugins which has invalid values in "name", e.g. not starting with "flipper-package-", or containing upper cased letters. Simple renaming of "name" field can be very problematic, so we need a new field to avoid any breaking changes and keep historical analytics data which is also bound to plugin id. Reviewed By: mweststrate Differential Revision: D21129689 fbshipit-source-id: efd143c82a6a802cc0b5438fd3f509bd99aded0e --- desktop/app/src/dispatcher/plugins.tsx | 10 +-- desktop/pkg-lib/src/PluginDetails.ts | 1 + .../src/__tests__/getPluginDetails.node.ts | 64 ++++++++++++++++++- desktop/pkg-lib/src/getPluginDetails.ts | 14 +++- desktop/plugins/network/index.tsx | 1 - desktop/plugins/network/package.json | 1 + 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/desktop/app/src/dispatcher/plugins.tsx b/desktop/app/src/dispatcher/plugins.tsx index 5c2260500..08a9214c1 100644 --- a/desktop/app/src/dispatcher/plugins.tsx +++ b/desktop/app/src/dispatcher/plugins.tsx @@ -165,15 +165,11 @@ export const requirePlugin = ( throw new Error(`Plugin ${plugin.name} is not a FlipperBasePlugin`); } + plugin.id = plugin.id || pluginDefinition.id; + // set values from package.json as static variables on class Object.keys(pluginDefinition).forEach((key) => { - if (key === 'name') { - plugin.id = plugin.id || pluginDefinition.name; - } else if (key === 'id') { - throw new Error( - 'Field "id" not allowed in package.json. The plugin\'s name will be used as ID"', - ); - } else { + if (key !== 'name' && key !== 'id') { plugin[key] = plugin[key] || pluginDefinition[key as keyof PluginDefinition]; } diff --git a/desktop/pkg-lib/src/PluginDetails.ts b/desktop/pkg-lib/src/PluginDetails.ts index ec3e3c0a3..43fdd50bb 100644 --- a/desktop/pkg-lib/src/PluginDetails.ts +++ b/desktop/pkg-lib/src/PluginDetails.ts @@ -14,6 +14,7 @@ export default interface PluginDetails { version: string; source: string; main: string; + id: string; gatekeeper?: string; icon?: string; title?: string; diff --git a/desktop/pkg-lib/src/__tests__/getPluginDetails.node.ts b/desktop/pkg-lib/src/__tests__/getPluginDetails.node.ts index dd5088389..dd9d2c9fd 100644 --- a/desktop/pkg-lib/src/__tests__/getPluginDetails.node.ts +++ b/desktop/pkg-lib/src/__tests__/getPluginDetails.node.ts @@ -31,6 +31,7 @@ test('getPluginDetailsV1', async () => { "dir": "./plugins/flipper-plugin-test", "gatekeeper": "GK_flipper_plugin_test", "icon": undefined, + "id": "flipper-plugin-test", "main": "dist/index.js", "name": "flipper-plugin-test", "source": "src/index.tsx", @@ -42,6 +43,66 @@ test('getPluginDetailsV1', async () => { }); test('getPluginDetailsV2', async () => { + const pluginV2 = { + specVersion: 2, + name: 'flipper-plugin-test', + title: 'Test', + version: '3.0.1', + main: 'dist/bundle.js', + flipperBundlerEntry: 'src/index.tsx', + gatekeeper: 'GK_flipper_plugin_test', + }; + fsMock.readJson.mockImplementation(() => pluginV2); + const details = await getPluginDetails('./plugins/flipper-plugin-test'); + expect(details).toMatchInlineSnapshot(` + Object { + "bugs": undefined, + "category": undefined, + "dir": "./plugins/flipper-plugin-test", + "gatekeeper": "GK_flipper_plugin_test", + "icon": undefined, + "id": "flipper-plugin-test", + "main": "dist/bundle.js", + "name": "flipper-plugin-test", + "source": "src/index.tsx", + "specVersion": 2, + "title": "Test", + "version": "3.0.1", + } + `); +}); + +test('id used as title if the latter omited', async () => { + const pluginV2 = { + specVersion: 2, + name: 'flipper-plugin-test', + id: 'test', + version: '3.0.1', + main: 'dist/bundle.js', + flipperBundlerEntry: 'src/index.tsx', + gatekeeper: 'GK_flipper_plugin_test', + }; + fsMock.readJson.mockImplementation(() => pluginV2); + const details = await getPluginDetails('./plugins/flipper-plugin-test'); + expect(details).toMatchInlineSnapshot(` + Object { + "bugs": undefined, + "category": undefined, + "dir": "./plugins/flipper-plugin-test", + "gatekeeper": "GK_flipper_plugin_test", + "icon": undefined, + "id": "test", + "main": "dist/bundle.js", + "name": "flipper-plugin-test", + "source": "src/index.tsx", + "specVersion": 2, + "title": "test", + "version": "3.0.1", + } + `); +}); + +test('name without "flipper-plugin-" prefix is used as title if the latter omited', async () => { const pluginV2 = { specVersion: 2, name: 'flipper-plugin-test', @@ -59,11 +120,12 @@ test('getPluginDetailsV2', async () => { "dir": "./plugins/flipper-plugin-test", "gatekeeper": "GK_flipper_plugin_test", "icon": undefined, + "id": "flipper-plugin-test", "main": "dist/bundle.js", "name": "flipper-plugin-test", "source": "src/index.tsx", "specVersion": 2, - "title": undefined, + "title": "test", "version": "3.0.1", } `); diff --git a/desktop/pkg-lib/src/getPluginDetails.ts b/desktop/pkg-lib/src/getPluginDetails.ts index ee2532687..acbf59694 100644 --- a/desktop/pkg-lib/src/getPluginDetails.ts +++ b/desktop/pkg-lib/src/getPluginDetails.ts @@ -42,9 +42,10 @@ async function getPluginDetailsV1( version: packageJson.version, main: path.join('dist', 'index.js'), source: packageJson.main, + id: packageJson.name, gatekeeper: packageJson.gatekeeper, icon: packageJson.icon, - title: packageJson.title, + title: packageJson.title || packageJson.name, category: packageJson.category, bugs: packageJson.bugs, }; @@ -62,10 +63,19 @@ async function getPluginDetailsV2( version: packageJson.version, main: packageJson.main, source: packageJson.flipperBundlerEntry, + id: packageJson.id || packageJson.name, gatekeeper: packageJson.gatekeeper, icon: packageJson.icon, - title: packageJson.displayName || packageJson.title, + title: + packageJson.title || packageJson.id || getTitleFromName(packageJson.name), category: packageJson.category, bugs: packageJson.bugs, }; } + +function getTitleFromName(name: string) { + const prefix = 'flipper-plugin-'; + if (name.startsWith(prefix)) { + return name.substr(prefix.length); + } +} diff --git a/desktop/plugins/network/index.tsx b/desktop/plugins/network/index.tsx index 4e831fe4a..b71b347a3 100644 --- a/desktop/plugins/network/index.tsx +++ b/desktop/plugins/network/index.tsx @@ -122,7 +122,6 @@ export const NetworkRouteContext = createContext( ); export default class extends FlipperPlugin { - static id = 'Network'; static keyboardActions: Array = ['clear']; static subscribed = []; static defaultPersistedState = { diff --git a/desktop/plugins/network/package.json b/desktop/plugins/network/package.json index b795baba3..f5104c6da 100644 --- a/desktop/plugins/network/package.json +++ b/desktop/plugins/network/package.json @@ -1,5 +1,6 @@ { "name": "flipper-plugin-network", + "id": "Network", "specVersion": 2, "flipperBundlerEntry": "index.tsx", "main": "dist/index.js",