From 92a743cc5946d8774563269b82f370e17df23e07 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 8 Dec 2021 04:25:28 -0800 Subject: [PATCH] Processed some earlier review comments Summary: Per title, processed some pending review comments made earlier in this stack Reviewed By: aigoncharov Differential Revision: D32916920 fbshipit-source-id: 01db85883596b5c85b77efc9cddadeac23cc4ef5 --- .../flipper-server-core/src/FlipperServerImpl.tsx | 9 +++++---- desktop/flipper-server-core/src/utils/keytar.tsx | 14 ++++++++------ .../src/utils/launcherSettings.tsx | 6 +++--- desktop/flipper-server/src/startSocketServer.tsx | 6 ++++++ desktop/flipper-server/src/startWebServerDev.tsx | 2 +- desktop/static/index.web.dev.html | 2 +- 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/desktop/flipper-server-core/src/FlipperServerImpl.tsx b/desktop/flipper-server-core/src/FlipperServerImpl.tsx index 89623349f..a21119631 100644 --- a/desktop/flipper-server-core/src/FlipperServerImpl.tsx +++ b/desktop/flipper-server-core/src/FlipperServerImpl.tsx @@ -33,7 +33,7 @@ import {launchEmulator} from './devices/android/AndroidDevice'; import {setFlipperServerConfig} from './FlipperServerConfig'; import {saveSettings} from './utils/settings'; import {saveLauncherSettings} from './utils/launcherSettings'; -import {KeytarManager} from './utils/keytar'; +import {KeytarManager, KeytarModule} from './utils/keytar'; import {PluginManager} from './plugins/PluginManager'; import {runHealthcheck, getHealthChecks} from './utils/runHealthchecks'; import {openFile} from './utils/openFile'; @@ -69,15 +69,16 @@ export class FlipperServerImpl implements FlipperServer { constructor( public config: FlipperServerConfig, public logger: Logger, - keytarModule?: any, + keytarModule?: KeytarModule, ) { setFlipperServerConfig(config); const server = (this.server = new ServerController(this)); this.android = new AndroidDeviceManager(this); this.ios = new IOSDeviceManager(this); this.keytarManager = new KeytarManager(keytarModule); - // TODO: given flipper-dump, it might make more sense to have the plugin command - // handled by moved to flipper-server & app, but let's keep things simple for now + // given flipper-dump, it might make more sense to have the plugin command + // handling (like download, install, etc) moved to flipper-server & app, + // but let's keep things simple for now this.pluginManager = new PluginManager(); server.addListener('error', (err) => { diff --git a/desktop/flipper-server-core/src/utils/keytar.tsx b/desktop/flipper-server-core/src/utils/keytar.tsx index 5cca22637..a3312288f 100644 --- a/desktop/flipper-server-core/src/utils/keytar.tsx +++ b/desktop/flipper-server-core/src/utils/keytar.tsx @@ -10,12 +10,14 @@ import os from 'os'; import {UserNotSignedInError} from 'flipper-common'; -export class KeytarManager { - keytar: any; +export type KeytarModule = { + getPassword(service: string, username: string): string; + deletePassword(service: string, username: string): void; + setPassword(service: string, username: string, password: string): void; +}; - constructor(keytarModule: any) { - this.keytar = keytarModule; - } +export class KeytarManager { + constructor(private keytar: KeytarModule | undefined) {} public async writeKeychain(service: string, password: string): Promise { if (this.keytar == null) { @@ -27,7 +29,7 @@ export class KeytarManager { } public async unsetKeychain(service: string): Promise { - await this.keytar.deletePassword(service, os.userInfo().username); + await this.keytar?.deletePassword(service, os.userInfo().username); } public async retrieveToken(service: string): Promise { diff --git a/desktop/flipper-server-core/src/utils/launcherSettings.tsx b/desktop/flipper-server-core/src/utils/launcherSettings.tsx index f40c7ca0e..2cc1b4279 100644 --- a/desktop/flipper-server-core/src/utils/launcherSettings.tsx +++ b/desktop/flipper-server-core/src/utils/launcherSettings.tsx @@ -14,6 +14,9 @@ import fs from 'fs-extra'; import TOML, {JsonMap} from '@iarna/toml'; import {LauncherSettings, ReleaseChannel} from 'flipper-common'; +// There is some disagreement among the XDG Base Directory implementations +// whether to use ~/Library/Preferences or ~/.config on MacOS. The Launcher +// expects the former, whereas `xdg-basedir` implements the latter. export function xdgConfigDir() { return os.platform() === 'darwin' ? path.join(os.homedir(), 'Library', 'Preferences') @@ -28,9 +31,6 @@ export function launcherConfigDir() { } function getLauncherSettingsFile(): string { - // There is some disagreement among the XDG Base Directory implementations - // whether to use ~/Library/Preferences or ~/.config on MacOS. The Launcher - // expects the former, whereas `xdg-basedir` implements the latter. return path.resolve(launcherConfigDir(), 'flipper-launcher.toml'); } diff --git a/desktop/flipper-server/src/startSocketServer.tsx b/desktop/flipper-server/src/startSocketServer.tsx index 19a862b09..1aa60a299 100644 --- a/desktop/flipper-server/src/startSocketServer.tsx +++ b/desktop/flipper-server/src/startSocketServer.tsx @@ -46,5 +46,11 @@ export function startSocketServer( connected = false; flipperServer.offAny(onServerEvent); }); + + client.on('error', (e) => { + console.error(chalk.red(`Socket error ${client.id}`), e); + connected = false; + flipperServer.offAny(onServerEvent); + }); }); } diff --git a/desktop/flipper-server/src/startWebServerDev.tsx b/desktop/flipper-server/src/startWebServerDev.tsx index 8cb7de413..f6e5a495d 100644 --- a/desktop/flipper-server/src/startWebServerDev.tsx +++ b/desktop/flipper-server/src/startWebServerDev.tsx @@ -119,7 +119,7 @@ async function startMetroServer( ) ).flat(), ); - // console.log('Source dirs\n\t' + watchFolders.join('\n\t')); + const baseConfig = await Metro.loadConfig(); const config = Object.assign({}, baseConfig, { projectRoot: rootDir, diff --git a/desktop/static/index.web.dev.html b/desktop/static/index.web.dev.html index 29f370150..26808f9b7 100644 --- a/desktop/static/index.web.dev.html +++ b/desktop/static/index.web.dev.html @@ -114,7 +114,7 @@ script.src = window.flipperConfig.entryPoint; script.onerror = (e) => { - openError('Script failure. Check Chrome console for more info. '); + openError('Script failure. Check Chrome console for more info.'); }; document.body.appendChild(script);