From 68275153298647224a7223461e5cdca9d19f8f73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20B=C3=BCchele?= Date: Thu, 20 Dec 2018 06:07:54 -0800 Subject: [PATCH] storing information about failed plugins Summary: If a plugin was not loaded we used to ignore it. This diff still ignores the plugins that weren't loaded, but adds some information about them to the redux store. This information is used in the PluginDebugger. These are the three reasons, why a plugin might not be loaded. * `gatekeepedPlugins`: Plugin was ignored because of a GK * `failedPlugins`: Plugin could not be requried/failed to parse * `disabledPlugins`: Plugin disabled in `~/.flipper/config.json` Information for each them is added to the redux store. Reviewed By: passy Differential Revision: D13516986 fbshipit-source-id: b7a55a159cb586d1a88fbb976248131c52a909c5 --- src/dispatcher/__tests__/plugins.electron.js | 29 +++++---- src/dispatcher/plugins.js | 63 +++++++++++++------ src/plugin.js | 2 + src/reducers/__tests__/plugins.node.js | 33 +++++++++- src/reducers/plugins.js | 65 ++++++++++++++++++-- 5 files changed, 155 insertions(+), 37 deletions(-) diff --git a/src/dispatcher/__tests__/plugins.electron.js b/src/dispatcher/__tests__/plugins.electron.js index 9557ea2df..5fede9b61 100644 --- a/src/dispatcher/__tests__/plugins.electron.js +++ b/src/dispatcher/__tests__/plugins.electron.js @@ -26,7 +26,7 @@ const logger = new Logger(); test('dispatcher dispatches REGISTER_PLUGINS', () => { dispatcher(mockStore, logger); const actions = mockStore.getActions(); - expect(actions[0].type).toBe('REGISTER_PLUGINS'); + expect(actions.map(a => a.type)).toContain('REGISTER_PLUGINS'); }); test('getDynamicPlugins returns empty array', () => { @@ -56,7 +56,7 @@ test('checkDisabled', () => { const config = {disabledPlugins: [disabledPlugin]}; // $FlowFixMe process.env not defined in electron API spec remote.process.env.CONFIG = JSON.stringify(config); - const disabled = checkDisabled(); + const disabled = checkDisabled([]); expect( disabled({ @@ -75,7 +75,7 @@ test('checkDisabled', () => { test('checkGK for plugin without GK', () => { expect( - checkGK({ + checkGK([])({ name: 'pluginID', out: './test/index.js', }), @@ -84,7 +84,7 @@ test('checkGK for plugin without GK', () => { test('checkGK for passing plugin', () => { expect( - checkGK({ + checkGK([])({ name: 'pluginID', gatekeeper: TEST_PASSING_GK, out: './test/index.js', @@ -93,17 +93,20 @@ test('checkGK for passing plugin', () => { }); test('checkGK for failing plugin', () => { - expect( - checkGK({ - name: 'pluginID', - gatekeeper: TEST_FAILING_GK, - out: './test/index.js', - }), - ).toBeFalsy(); + const gatekeepedPlugins = []; + const name = 'pluginID'; + const plugins = checkGK(gatekeepedPlugins)({ + name, + gatekeeper: TEST_FAILING_GK, + out: './test/index.js', + }); + + expect(plugins).toBeFalsy(); + expect(gatekeepedPlugins[0].name).toEqual(name); }); test('requirePlugin returns null for invalid requires', () => { - const plugin = requirePlugin(require)({ + const plugin = requirePlugin([], require)({ name: 'pluginID', out: 'this/path/does not/exist', }); @@ -114,7 +117,7 @@ test('requirePlugin returns null for invalid requires', () => { test('requirePlugin loads plugin', () => { const name = 'pluginID'; const homepage = 'https://fb.workplace.com/groups/230455004101832/'; - const plugin = requirePlugin(require)({ + const plugin = requirePlugin([], require)({ name, homepage, out: path.join(__dirname, 'TestPlugin.js'), diff --git a/src/dispatcher/plugins.js b/src/dispatcher/plugins.js index 5eaa025bd..1a20fa514 100644 --- a/src/dispatcher/plugins.js +++ b/src/dispatcher/plugins.js @@ -13,16 +13,22 @@ import type {State} from '../reducers/plugins'; import React from 'react'; import ReactDOM from 'react-dom'; import * as Flipper from 'flipper'; -import {registerPlugins} from '../reducers/plugins'; +import { + registerPlugins, + addGatekeepedPlugins, + addDisabledPlugins, + addFailedPlugins, +} from '../reducers/plugins'; import {remote} from 'electron'; import {GK} from 'flipper'; import {FlipperBasePlugin} from '../plugin.js'; import {setupMenuBar} from '../MenuBar.js'; -type PluginDefinition = { +export type PluginDefinition = { name: string, out: string, gatekeeper?: string, + entry?: string, }; export default (store: Store, logger: Logger) => { @@ -31,16 +37,21 @@ export default (store: Store, logger: Logger) => { window.ReactDOM = ReactDOM; window.Flipper = Flipper; - const disabled = checkDisabled(); + const gatekeepedPlugins: Array = []; + const disabledPlugins: Array = []; + const failedPlugins: Array<[PluginDefinition, string]> = []; const initialPlugins: Array< Class | FlipperDevicePlugin<>>, > = [...getBundledPlugins(), ...getDynamicPlugins()] - .filter(disabled) - .filter(checkGK) - .map(requirePlugin()) + .filter(checkDisabled(disabledPlugins)) + .filter(checkGK(gatekeepedPlugins)) + .map(requirePlugin(failedPlugins)) .filter(Boolean); + store.dispatch(addGatekeepedPlugins(gatekeepedPlugins)); + store.dispatch(addDisabledPlugins(disabledPlugins)); + store.dispatch(addFailedPlugins(failedPlugins)); store.dispatch(registerPlugins(initialPlugins)); let state: ?State = null; @@ -81,22 +92,30 @@ export function getDynamicPlugins() { return dynamicPlugins; } -export function checkGK(plugin: PluginDefinition): boolean { - const result = plugin.gatekeeper && !GK.get(plugin.gatekeeper); - if (plugin.gatekeeper && !result) { +export const checkGK = (gatekeepedPlugins: Array) => ( + plugin: PluginDefinition, +): boolean => { + if (!plugin.gatekeeper) { + return true; + } + const result = GK.get(plugin.gatekeeper); + if (!result) { + gatekeepedPlugins.push(plugin); console.warn( 'Plugin %s will be ignored as user is not part of the gatekeeper "%s".', plugin.name, plugin.gatekeeper, ); } - return !result; -} + return result; +}; -export function checkDisabled(): (plugin: PluginDefinition) => boolean { - let disabledPlugins: Set = new Set(); +export const checkDisabled = (disabledPlugins: Array) => ( + plugin: PluginDefinition, +): boolean => { + let disabledList: Set = new Set(); try { - disabledPlugins = new Set( + disabledList = new Set( // $FlowFixMe process.env not defined in electron API spec JSON.parse(remote?.process.env.CONFIG || '{}').disabledPlugins || [], ); @@ -104,12 +123,17 @@ export function checkDisabled(): (plugin: PluginDefinition) => boolean { console.error(e); } - return (plugin: PluginDefinition) => !disabledPlugins.has(plugin.name); -} + if (disabledList.has(plugin.name)) { + disabledPlugins.push(plugin); + } -export function requirePlugin( + return !disabledList.has(plugin.name); +}; + +export const requirePlugin = ( + failedPlugins: Array<[PluginDefinition, string]>, requireFunction: Function = window.electronRequire, -) { +) => { return ( pluginDefinition: PluginDefinition, ): ?Class | FlipperDevicePlugin<>> => { @@ -137,8 +161,9 @@ export function requirePlugin( return plugin; } catch (e) { + failedPlugins.push([pluginDefinition, e.message]); console.error(pluginDefinition, e); return null; } }; -} +}; diff --git a/src/plugin.js b/src/plugin.js index ef2a16960..26a6d796c 100644 --- a/src/plugin.js +++ b/src/plugin.js @@ -52,6 +52,8 @@ export class FlipperBasePlugin< static title: ?string = null; static id: string = ''; static icon: ?string = null; + static gatekeeper: ?string = null; + static entry: ?string = null; static bugs: ?{ email?: string, url?: string, diff --git a/src/reducers/__tests__/plugins.node.js b/src/reducers/__tests__/plugins.node.js index 221898603..c73ceadee 100644 --- a/src/reducers/__tests__/plugins.node.js +++ b/src/reducers/__tests__/plugins.node.js @@ -5,7 +5,11 @@ * @format */ -import {default as reducer, registerPlugins} from '../plugins'; +import { + default as reducer, + registerPlugins, + addGatekeepedPlugins, +} from '../plugins'; import { FlipperBasePlugin, FlipperPlugin, @@ -29,6 +33,9 @@ test('add clientPlugin', () => { { devicePlugins: new Map(), clientPlugins: new Map(), + gatekeepedPlugins: [], + failedPlugins: [], + disabledPlugins: [], }, registerPlugins([testPlugin]), ); @@ -40,6 +47,9 @@ test('add devicePlugin', () => { { devicePlugins: new Map(), clientPlugins: new Map(), + gatekeepedPlugins: [], + failedPlugins: [], + disabledPlugins: [], }, registerPlugins([testDevicePlugin]), ); @@ -51,6 +61,9 @@ test('do not add plugin twice', () => { { devicePlugins: new Map(), clientPlugins: new Map(), + gatekeepedPlugins: [], + failedPlugins: [], + disabledPlugins: [], }, registerPlugins([testPlugin, testPlugin]), ); @@ -62,6 +75,9 @@ test('do not add other classes', () => { { devicePlugins: new Map(), clientPlugins: new Map(), + gatekeepedPlugins: [], + failedPlugins: [], + disabledPlugins: [], }, // $FlowFixMe testing wrong classes on purpose here registerPlugins([testBasePlugin]), @@ -69,3 +85,18 @@ test('do not add other classes', () => { expect(res.devicePlugins.size).toEqual(0); expect(res.devicePlugins.size).toEqual(0); }); + +test('add gatekeeped plugin', () => { + const gatekeepedPlugins = [{name: 'plugin', out: 'out.js'}]; + const res = reducer( + { + devicePlugins: new Map(), + clientPlugins: new Map(), + gatekeepedPlugins: [], + failedPlugins: [], + disabledPlugins: [], + }, + addGatekeepedPlugins(gatekeepedPlugins), + ); + expect(res.gatekeepedPlugins).toEqual(gatekeepedPlugins); +}); diff --git a/src/reducers/plugins.js b/src/reducers/plugins.js index ddcb31751..cbb88d83f 100644 --- a/src/reducers/plugins.js +++ b/src/reducers/plugins.js @@ -7,21 +7,42 @@ import {FlipperPlugin, FlipperDevicePlugin} from '../plugin.js'; +import type {PluginDefinition} from '../dispatcher/plugins'; + export type State = { devicePlugins: Map>>, clientPlugins: Map>>, + gatekeepedPlugins: Array, + disabledPlugins: Array, + failedPlugins: Array<[PluginDefinition, string]>, }; type P = Class | FlipperDevicePlugin<>>; -export type Action = { - type: 'REGISTER_PLUGINS', - payload: Array

, -}; +export type Action = + | { + type: 'REGISTER_PLUGINS', + payload: Array

, + } + | { + type: 'GATEKEEPED_PLUGINS', + payload: Array, + } + | { + type: 'DISABLED_PLUGINS', + payload: Array, + } + | { + type: 'FAILED_PLUGINS', + payload: Array<[PluginDefinition, string]>, + }; const INITIAL_STATE: State = { devicePlugins: new Map(), clientPlugins: new Map(), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], }; export default function reducer( @@ -51,6 +72,21 @@ export default function reducer( devicePlugins, clientPlugins, }; + } else if (action.type === 'GATEKEEPED_PLUGINS') { + return { + ...state, + gatekeepedPlugins: state.gatekeepedPlugins.concat(action.payload), + }; + } else if (action.type === 'DISABLED_PLUGINS') { + return { + ...state, + disabledPlugins: state.disabledPlugins.concat(action.payload), + }; + } else if (action.type === 'FAILED_PLUGINS') { + return { + ...state, + failedPlugins: state.failedPlugins.concat(action.payload), + }; } else { return state; } @@ -60,3 +96,24 @@ export const registerPlugins = (payload: Array

): Action => ({ type: 'REGISTER_PLUGINS', payload, }); + +export const addGatekeepedPlugins = ( + payload: Array, +): Action => ({ + type: 'GATEKEEPED_PLUGINS', + payload, +}); + +export const addDisabledPlugins = ( + payload: Array, +): Action => ({ + type: 'DISABLED_PLUGINS', + payload, +}); + +export const addFailedPlugins = ( + payload: Array<[PluginDefinition, string]>, +): Action => ({ + type: 'FAILED_PLUGINS', + payload, +});