From f43ff735911ec5f59023f6ab07dd4db0335c38b5 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 14 Oct 2021 03:16:38 -0700 Subject: [PATCH] Addressed some earlier diff comments Summary: Async processed review feedback from stack D31474919 (https://github.com/facebook/flipper/commit/cfd44b592a977b61042b52917415c3c00351e73e) e.o. (decapitate) Reviewed By: passy Differential Revision: D31608309 fbshipit-source-id: 536b3c9350f7acc40530000ecf5e46d5b074d50f --- desktop/app/src/dispatcher/flipperServer.tsx | 27 ++++++++++-------- desktop/flipper-dump/src/index.tsx | 28 ++++++++++--------- .../src/FlipperServerImpl.tsx | 11 +++++--- .../devices/ios/__tests__/iOSDevice.node.tsx | 11 +++++--- desktop/flipper-server-core/src/index.tsx | 6 ---- 5 files changed, 44 insertions(+), 39 deletions(-) diff --git a/desktop/app/src/dispatcher/flipperServer.tsx b/desktop/app/src/dispatcher/flipperServer.tsx index daefe6d10..27ad23cbe 100644 --- a/desktop/app/src/dispatcher/flipperServer.tsx +++ b/desktop/app/src/dispatcher/flipperServer.tsx @@ -10,7 +10,7 @@ import React from 'react'; import {State, Store} from '../reducers/index'; import {Logger} from 'flipper-common'; -import {FlipperServerImpl, setFlipperServerConfig} from 'flipper-server-core'; +import {FlipperServerImpl} from 'flipper-server-core'; import {selectClient} from '../reducers/connections'; import Client from '../Client'; import {notification} from 'antd'; @@ -24,17 +24,20 @@ import constants from '../fb-stubs/constants'; export default async (store: Store, logger: Logger) => { const {enableAndroid, androidHome, idbPath, enableIOS, enablePhysicalIOS} = store.getState().settingsState; - setFlipperServerConfig({ - enableAndroid, - androidHome, - idbPath, - enableIOS, - enablePhysicalIOS, - staticPath: getStaticPath(), - tmpPath: getAppTempPath(), - validWebSocketOrigins: constants.VALID_WEB_SOCKET_REQUEST_ORIGIN_PREFIXES, - }); - const server = new FlipperServerImpl(logger); + + const server = new FlipperServerImpl( + { + enableAndroid, + androidHome, + idbPath, + enableIOS, + enablePhysicalIOS, + staticPath: getStaticPath(), + tmpPath: getAppTempPath(), + validWebSocketOrigins: constants.VALID_WEB_SOCKET_REQUEST_ORIGIN_PREFIXES, + }, + logger, + ); store.dispatch({ type: 'SET_FLIPPER_SERVER', diff --git a/desktop/flipper-dump/src/index.tsx b/desktop/flipper-dump/src/index.tsx index 215a42abc..06cb41135 100644 --- a/desktop/flipper-dump/src/index.tsx +++ b/desktop/flipper-dump/src/index.tsx @@ -10,7 +10,7 @@ import fs from 'fs'; import os from 'os'; import yargs from 'yargs'; -import {FlipperServerImpl, setFlipperServerConfig} from 'flipper-server-core'; +import {FlipperServerImpl} from 'flipper-server-core'; import { ClientDescription, Logger, @@ -63,20 +63,22 @@ async function start(deviceTitle: string, appName: string, pluginId: string) { console.debug = () => {}; console.info = console.error; - // TODO: make these better overridable - setFlipperServerConfig({ - enableAndroid: true, - androidHome: process.env.ANDROID_HOME || '/opt/android_sdk', - idbPath: '/usr/local/bin/idb', - enableIOS: true, - enablePhysicalIOS: true, - staticPath: path.resolve(__dirname, '..', '..', 'static'), - tmpPath: os.tmpdir(), - validWebSocketOrigins: [], - }); // TODO: initialise FB user manager to be able to do certificate exchange - const server = new FlipperServerImpl(logger); + const server = new FlipperServerImpl( + { + // TODO: make these better overridable + enableAndroid: true, + androidHome: process.env.ANDROID_HOME || '/opt/android_sdk', + idbPath: '/usr/local/bin/idb', + enableIOS: true, + enablePhysicalIOS: true, + staticPath: path.resolve(__dirname, '..', '..', 'static'), + tmpPath: os.tmpdir(), + validWebSocketOrigins: [], + }, + logger, + ); logger.info( `Waiting for device '${deviceTitle}' client '${appName}' plugin '${pluginId}' ...`, diff --git a/desktop/flipper-server-core/src/FlipperServerImpl.tsx b/desktop/flipper-server-core/src/FlipperServerImpl.tsx index 1d7833433..40bccc9b5 100644 --- a/desktop/flipper-server-core/src/FlipperServerImpl.tsx +++ b/desktop/flipper-server-core/src/FlipperServerImpl.tsx @@ -29,7 +29,10 @@ import {ServerDevice} from './devices/ServerDevice'; import {Base64} from 'js-base64'; import MetroDevice from './devices/metro/MetroDevice'; import {launchEmulator} from './devices/android/AndroidDevice'; -import {getFlipperServerConfig} from './FlipperServerConfig'; +import { + FlipperServerConfig, + setFlipperServerConfig, +} from './FlipperServerConfig'; /** * FlipperServer takes care of all incoming device & client connections. @@ -49,8 +52,8 @@ export class FlipperServerImpl implements FlipperServer { android: AndroidDeviceManager; ios: IOSDeviceManager; - constructor(public logger: Logger) { - getFlipperServerConfig(); // Config should be available at this point! + constructor(config: FlipperServerConfig, public logger: Logger) { + setFlipperServerConfig(config); const server = (this.server = new ServerController(this)); this.android = new AndroidDeviceManager(this); this.ios = new IOSDeviceManager(this); @@ -89,7 +92,7 @@ export class FlipperServerImpl implements FlipperServer { title: `Timed out establishing connection with "${client.appName}" on "${client.deviceName}".`, description: medium === 'WWW' - ? `Verify that both your computer and mobile device are on Lighthouse/VPN that you are logged in to Facebook Intern so that certificates can be exhanged. See: https://www.internalfb.com/intern/wiki/Ops/Network/Enterprise_Network_Engineering/ene_wlra/VPN_Help/Vpn/mobile/` + ? `Verify that both your computer and mobile device are on Lighthouse/VPN that you are logged in to Facebook Intern so that certificates can be exhanged. See: https://fburl.com/flippervpn` : 'Verify that your client is connected to Flipper and that there is no error related to idb.', }); }, diff --git a/desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx b/desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx index d252ba8ec..a0658b274 100644 --- a/desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx +++ b/desktop/flipper-server-core/src/devices/ios/__tests__/iOSDevice.node.tsx @@ -11,6 +11,9 @@ import {parseXcodeFromCoreSimPath} from '../iOSDeviceManager'; import {getLogger} from 'flipper-common'; import {IOSBridge} from '../IOSBridge'; import {FlipperServerImpl} from '../../../FlipperServerImpl'; +import {getFlipperServerConfig} from '../../../FlipperServerConfig'; + +const testConfig = getFlipperServerConfig(); const standardCoresimulatorLog = 'username 1264 0.0 0.1 5989740 41648 ?? Ss 2:23PM 0:12.92 /Applications/Xcode_12.4.0_fb.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/usr/libexec/mobileassetd'; @@ -53,7 +56,7 @@ test('test parseXcodeFromCoreSimPath from standard locations', () => { }); test('test getAllPromisesForQueryingDevices when xcode detected', () => { - const flipperServer = new FlipperServerImpl(getLogger()); + const flipperServer = new FlipperServerImpl(testConfig, getLogger()); flipperServer.ios.iosBridge = {} as IOSBridge; const promises = flipperServer.ios.getAllPromisesForQueryingDevices( true, @@ -63,7 +66,7 @@ test('test getAllPromisesForQueryingDevices when xcode detected', () => { }); test('test getAllPromisesForQueryingDevices when xcode is not detected', () => { - const flipperServer = new FlipperServerImpl(getLogger()); + const flipperServer = new FlipperServerImpl(testConfig, getLogger()); flipperServer.ios.iosBridge = {} as IOSBridge; const promises = flipperServer.ios.getAllPromisesForQueryingDevices( false, @@ -73,7 +76,7 @@ test('test getAllPromisesForQueryingDevices when xcode is not detected', () => { }); test('test getAllPromisesForQueryingDevices when xcode and idb are both unavailable', () => { - const flipperServer = new FlipperServerImpl(getLogger()); + const flipperServer = new FlipperServerImpl(testConfig, getLogger()); flipperServer.ios.iosBridge = {} as IOSBridge; const promises = flipperServer.ios.getAllPromisesForQueryingDevices( false, @@ -83,7 +86,7 @@ test('test getAllPromisesForQueryingDevices when xcode and idb are both unavaila }); test('test getAllPromisesForQueryingDevices when both idb and xcode are available', () => { - const flipperServer = new FlipperServerImpl(getLogger()); + const flipperServer = new FlipperServerImpl(testConfig, getLogger()); flipperServer.ios.iosBridge = {} as IOSBridge; const promises = flipperServer.ios.getAllPromisesForQueryingDevices( true, diff --git a/desktop/flipper-server-core/src/index.tsx b/desktop/flipper-server-core/src/index.tsx index 332550fd7..d802e22cd 100644 --- a/desktop/flipper-server-core/src/index.tsx +++ b/desktop/flipper-server-core/src/index.tsx @@ -7,10 +7,4 @@ * @format */ -export { - FlipperServerConfig, - getFlipperServerConfig, - setFlipperServerConfig, -} from './FlipperServerConfig'; - export {FlipperServerImpl} from './FlipperServerImpl';