Cleanup logger

Summary: Some cleanup around Logger, like renaming `getInstance` -> `getLogger`

Reviewed By: aigoncharov

Differential Revision: D31480828

fbshipit-source-id: d2fa9dc2b45bc34d846d45e396a74b80906d3ad5

desktop/app/src/PluginContainer.tsx

@@ -8,7 +8,7 @@
  */
 
 import {FlipperPlugin, FlipperDevicePlugin} from './plugin';
-import {Logger} from 'flipper-common';
+import {Logger, isTest} from 'flipper-common';
 import BaseDevice from './devices/BaseDevice';
 import {pluginKey as getPluginKey} from './utils/pluginKey';
 import Client from './Client';
@@ -49,7 +49,6 @@ import {produce} from 'immer';
 import {reportUsage} from './utils/metrics';
 import {PluginInfo} from './chrome/fb-stubs/PluginInfo';
 import {getActiveClient, getActivePlugin} from './selectors/connections';
-import {isTest} from './utils/isProduction';
 import {AnyAction} from 'redux';
 
 const {Text, Link} = Typography;

desktop/app/src/chrome/ShareSheetExportUrl.tsx

@@ -26,7 +26,7 @@ import ShareSheetErrorList from './ShareSheetErrorList';
 import {reportPlatformFailures} from '../utils/metrics';
 import {performance} from 'perf_hooks';
 import ShareSheetPendingDialog from './ShareSheetPendingDialog';
-import {getInstance as getLogger} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {resetSupportFormV2State} from '../reducers/supportForm';
 import {MiddlewareAPI} from '../reducers/index';
 import {getFlipperLib, Layout} from 'flipper-plugin';

desktop/app/src/dispatcher/__tests__/plugins.node.tsx

@@ -22,7 +22,7 @@ import path from 'path';
 // eslint-disable-next-line flipper/no-electron-remote-imports
 import {remote} from 'electron';
 import {createRootReducer, State} from '../../reducers/index';
-import {getInstance} from '../../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import configureStore from 'redux-mock-store';
 import {TEST_PASSING_GK, TEST_FAILING_GK} from '../../fb-stubs/GK';
 import TestPlugin from './TestPlugin';
@@ -36,7 +36,7 @@ const loadDynamicPluginsMock = mocked(loadDynamicPlugins);
 const mockStore = configureStore<State, {}>([])(
   createRootReducer()(undefined, {type: 'INIT'}),
 );
-const logger = getInstance();
+const logger = getLogger();
 
 const sampleInstalledPluginDetails: InstalledPluginDetails = {
   name: 'other Name',

desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx

@@ -9,6 +9,7 @@
 
 import React from 'react';
 import {Dialog, getFlipperLib} from 'flipper-plugin';
+import {isTest} from 'flipper-common';
 import {getUser} from '../fb-stubs/user';
 import {State, Store} from '../reducers/index';
 import {checkForUpdate} from '../fb-stubs/checkForUpdate';
@@ -21,7 +22,7 @@ import {getPluginStatus, PluginStatus} from '../utils/pluginUtils';
 import {loadPluginsFromMarketplace} from './fb-stubs/pluginMarketplace';
 import {loadPlugin, switchPlugin} from '../reducers/pluginManager';
 import {startPluginDownload} from '../reducers/pluginDownloads';
-import isProduction, {isTest} from '../utils/isProduction';
+import isProduction from '../utils/isProduction';
 import restart from '../utils/restartFlipper';
 import BaseDevice from '../devices/BaseDevice';
 import Client from '../Client';

desktop/app/src/fb-stubs/Logger.tsx

@@ -7,23 +7,9 @@
  * @format
  */
 
-import {Logger, LoggerArgs} from 'flipper-common';
-import StubLogger from '../utils/StubLogger';
+import {Logger, LoggerArgs, NoopLogger} from 'flipper-common';
 import {Store} from '../reducers/index';
 
-let instance: StubLogger | null = null;
-
-export function init(store: Store, _args?: LoggerArgs): Logger {
-  if (instance) {
-    throw new Error('Attempted to initialize Logger when already initialized');
-  }
-  instance = new StubLogger(store, _args);
-  return instance;
-}
-
-export function getInstance(): Logger {
-  if (!instance) {
-    return init(undefined as any /* store is not actually used */);
-  }
-  return instance;
+export function init(_store: Store, _args?: LoggerArgs): Logger {
+  return new NoopLogger();
 }

desktop/app/src/index.tsx

@@ -124,7 +124,7 @@ export {KeyboardActions} from './MenuBar';
 export {getFlipperMediaCDN, appendAccessTokenToUrl} from './fb-stubs/user';
 export {Rect} from './utils/geometry';
 export {Logger} from 'flipper-common';
-export {getInstance as getLogger} from './fb-stubs/Logger';
+export {getLogger} from 'flipper-common';
 export {callVSCode} from './utils/vscodeUtils';
 export {checkIdbIsInstalled} from './server/devices/ios/iOSContainerUtility';
 export {IDEFileResolver, IDEType} from './fb-stubs/IDEFileResolver';

desktop/app/src/init.tsx

@@ -52,6 +52,7 @@ import {PersistGate} from 'redux-persist/integration/react';
 // eslint-disable-next-line flipper/no-electron-remote-imports
 import {ipcRenderer, remote} from 'electron';
 import {helloWorld} from 'flipper-server-core';
+import {setLoggerInstance} from 'flipper-common';
 
 helloWorld();
 
@@ -186,6 +187,7 @@ function setProcessState(store: Store) {
 function init() {
   const store = getStore();
   const logger = initLogger(store);
+  setLoggerInstance(logger);
 
   // rehydrate app state before exposing init
   const persistor = persistStore(store, undefined, () => {

desktop/app/src/reducers/supportForm.tsx

@@ -15,7 +15,7 @@ import {showStatusUpdatesForDuration} from '../utils/promiseTimeout';
 import {selectedPlugins as setSelectedPlugins} from './plugins';
 import {addStatusMessage, removeStatusMessage} from './application';
 import constants from '../fb-stubs/constants';
-import {getInstance} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {logPlatformSuccessRate} from '../utils/metrics';
 export const SUPPORT_FORM_PREFIX = 'support-form-v2';
 import {getExportablePlugins} from '../selectors/connections';
@@ -109,7 +109,7 @@ export class Group {
   }
 
   handleSupportFormDeeplinks(store: Store) {
-    getInstance().track('usage', 'support-form-source', {
+    getLogger().track('usage', 'support-form-source', {
       source: 'deeplink',
       group: this.name,
     });

desktop/app/src/sandy-chrome/LeftRail.tsx

@@ -40,7 +40,7 @@ import styled from '@emotion/styled';
 import {showEmulatorLauncher} from './appinspect/LaunchEmulator';
 import SupportRequestFormV2 from '../fb-stubs/SupportRequestFormV2';
 import {setStaticView, StaticView} from '../reducers/connections';
-import {getInstance} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {SandyRatingButton} from '../chrome/RatingButton';
 import {filterNotifications} from './notification/notificationUtils';
 import {useMemoize} from 'flipper-plugin';
@@ -316,7 +316,7 @@ function SupportFormButton() {
       title="Feedback / Bug Reporter"
       selected={isStaticViewActive(staticView, SupportRequestFormV2)}
       onClick={() => {
-        getInstance().track('usage', 'support-form-source', {
+        getLogger().track('usage', 'support-form-source', {
           source: 'sidebar',
           group: undefined,
         });

desktop/app/src/server/comms/ServerController.tsx

@@ -9,7 +9,7 @@
 
 import {CertificateExchangeMedium} from '../utils/CertificateProvider';
 import {Logger} from 'flipper-common';
-import {ClientDescription, ClientQuery} from 'flipper-common';
+import {ClientDescription, ClientQuery, isTest} from 'flipper-common';
 import CertificateProvider from '../utils/CertificateProvider';
 import {ClientConnection, ConnectionStatus} from './ClientConnection';
 import {UninitializedClient} from 'flipper-common';
@@ -33,7 +33,6 @@ import {
   TransportType,
 } from './ServerFactory';
 import {FlipperServerImpl} from '../FlipperServerImpl';
-import {isTest} from '../../utils/isProduction';
 
 type ClientInfo = {
   connection: ClientConnection | null | undefined;

desktop/app/src/server/devices/ios/__tests__/iOSDevice.node.tsx

@@ -10,7 +10,7 @@
 import {parseXcodeFromCoreSimPath} from '../iOSDeviceManager';
 import configureStore from 'redux-mock-store';
 import {State, createRootReducer} from '../../../../reducers/index';
-import {getInstance} from '../../../../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {IOSBridge} from '../IOSBridge';
 import {FlipperServerImpl} from '../../../FlipperServerImpl';
 
@@ -59,7 +59,7 @@ test('test parseXcodeFromCoreSimPath from standard locations', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode detected', () => {
-  const flipperServer = new FlipperServerImpl({}, getInstance());
+  const flipperServer = new FlipperServerImpl({}, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     true,
@@ -69,7 +69,7 @@ test('test getAllPromisesForQueryingDevices when xcode detected', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode is not detected', () => {
-  const flipperServer = new FlipperServerImpl({}, getInstance());
+  const flipperServer = new FlipperServerImpl({}, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     false,
@@ -79,7 +79,7 @@ test('test getAllPromisesForQueryingDevices when xcode is not detected', () => {
 });
 
 test('test getAllPromisesForQueryingDevices when xcode and idb are both unavailable', () => {
-  const flipperServer = new FlipperServerImpl({}, getInstance());
+  const flipperServer = new FlipperServerImpl({}, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     false,
@@ -89,7 +89,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({}, getInstance());
+  const flipperServer = new FlipperServerImpl({}, getLogger());
   flipperServer.ios.iosBridge = {} as IOSBridge;
   const promises = flipperServer.ios.getAllPromisesForQueryingDevices(
     true,

desktop/app/src/server/utils/CertificateProvider.tsx

@@ -26,10 +26,8 @@ import * as androidUtil from '../devices/android/androidContainerUtility';
 import os from 'os';
 import {Client as ADBClient} from 'adbkit';
 import archiver from 'archiver';
-import {timeout} from 'flipper-common';
+import {timeout, isTest} from 'flipper-common';
 import {v4 as uuid} from 'uuid';
-import {isTest} from '../../utils/isProduction';
-import {message} from 'antd';
 
 export type CertificateExchangeMedium = 'FS_ACCESS' | 'WWW' | 'NONE';
 

desktop/app/src/test-utils/MockFlipper.tsx

@@ -20,7 +20,7 @@ import {buildClientId} from '../utils/clientUtils';
 import {Logger} from 'flipper-common';
 import {PluginDefinition} from '../plugin';
 import {registerPlugins} from '../reducers/plugins';
-import {getInstance} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {initializeFlipperLibImplementation} from '../utils/flipperLibImplementation';
 import pluginManager from '../dispatcher/pluginManager';
 import {PluginDetails} from 'flipper-plugin-lib';
@@ -83,7 +83,7 @@ export default class MockFlipper {
 
   public async init({plugins}: AppOptions = {}) {
     this._store = createStore(createRootReducer());
-    this._logger = getInstance();
+    this._logger = getLogger();
     this.unsubscribePluginManager = pluginManager(this._store, this._logger, {
       runSideEffectsSynchronously: true,
     });

desktop/app/src/utils/StubLogger.tsx

@@ -1,32 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @format
- */
-
-import {Logger, LoggerTrackType, LoggerArgs} from 'flipper-common';
-import {Store} from '../reducers/index';
-
-export default class StubLogger implements Logger {
-  constructor(_store: Store, _args?: LoggerArgs) {}
-
-  track(
-    _type: LoggerTrackType,
-    _event: string,
-    _data?: any,
-    _plugin?: string,
-  ) {}
-
-  trackTimeSince(_mark: string, _eventName?: string) {}
-
-  info(_data: any, _category: string) {}
-
-  warn(_data: any, _category: string) {}
-
-  error(_data: any, _category: string) {}
-
-  debug(_data: any, _category: string) {}
-}

desktop/app/src/utils/__tests__/info.node.tsx

@@ -13,7 +13,7 @@ import {createRootReducer} from '../../reducers';
 import initialize, {getInfo} from '../info';
 import {registerLoadedPlugins} from '../../reducers/plugins';
 import {TestUtils} from 'flipper-plugin';
-import {getInstance} from '../../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {selectPlugin} from '../../reducers/connections';
 import {renderMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin';
 
@@ -64,7 +64,7 @@ describe('info', () => {
         additionalPlugins: [inspectorPlugin],
       },
     );
-    initialize(store, getInstance());
+    initialize(store, getLogger());
     store.dispatch(
       registerLoadedPlugins([networkPluginDetails, inspectorPluginDetails]),
     );

desktop/app/src/utils/exportData.tsx

@@ -11,7 +11,7 @@ import * as React from 'react';
 import os from 'os';
 import path from 'path';
 import electron from 'electron';
-import {getInstance, getInstance as getLogger} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {Store, MiddlewareAPI} from '../reducers';
 import {DeviceExport} from '../devices/BaseDevice';
 import {selectedPlugins, State as PluginsState} from '../reducers/plugins';
@@ -637,7 +637,7 @@ export async function startFileExport(dispatch: Store['dispatch']) {
   // need to be cleaned up later in combination with SupportForm
   dispatch(selectedPlugins(plugins));
   Dialog.showModal((onHide) => (
-    <ShareSheetExportFile onHide={onHide} file={file} logger={getInstance()} />
+    <ShareSheetExportFile onHide={onHide} file={file} logger={getLogger()} />
   ));
 }
 
@@ -650,7 +650,7 @@ export async function startLinkExport(dispatch: Store['dispatch']) {
   // need to be cleaned up later in combination with SupportForm
   dispatch(selectedPlugins(plugins));
   Dialog.showModal((onHide) => (
-    <ShareSheetExportUrl onHide={onHide} logger={getInstance()} />
+    <ShareSheetExportUrl onHide={onHide} logger={getLogger()} />
   ));
 }
 

desktop/app/src/utils/info.tsx

@@ -8,13 +8,13 @@
  */
 
 import os from 'os';
-import isProduction, {isTest} from './isProduction';
+import isProduction from './isProduction';
 import fs from 'fs-extra';
 import {getStaticPath} from './pathUtils';
 import type {State, Store} from '../reducers/index';
 import {deconstructClientId} from './clientUtils';
 import {sideEffect} from './sideEffect';
-import {Logger} from 'flipper-common';
+import {Logger, isTest} from 'flipper-common';
 
 type PlatformInfo = {
   arch: string;

desktop/app/src/utils/isProduction.tsx

@@ -15,12 +15,6 @@ const _isProduction = !/node_modules[\\/]electron[\\/]/.test(
   process.execPath || electron.remote.process.execPath,
 );
 
-const _isTest = !!process.env.JEST_WORKER_ID;
-
 export default function isProduction(): boolean {
   return _isProduction;
 }
-
-export function isTest(): boolean {
-  return _isTest;
-}

desktop/app/src/utils/metrics.tsx

@@ -7,7 +7,7 @@
  * @format
  */
 
-import {getInstance} from '../fb-stubs/Logger';
+import {getLogger} from 'flipper-common';
 import {CancelledPromiseError, isError} from './errors';
 
 type Result =
@@ -144,16 +144,16 @@ export function reportUsage(
   data?: {[key: string]: string},
   plugin?: string,
 ) {
-  getInstance().track('usage', action, data, plugin);
+  getLogger().track('usage', action, data, plugin);
 }
 
 export function logPlatformSuccessRate(name: string, result: Result) {
   if (result.kind === 'success') {
-    getInstance().track('success-rate', name, {value: 1});
+    getLogger().track('success-rate', name, {value: 1});
   } else if (result.kind === 'cancelled') {
-    getInstance().track('operation-cancelled', name);
+    getLogger().track('operation-cancelled', name);
   } else {
-    getInstance().track('success-rate', name, {
+    getLogger().track('success-rate', name, {
       value: 0,
       supportedOperation: result.supportedOperation ? 1 : 0,
       error: extractMessage(result.error),
@@ -163,11 +163,11 @@ export function logPlatformSuccessRate(name: string, result: Result) {
 
 function logPluginSuccessRate(name: string, plugin: string, result: Result) {
   if (result.kind === 'success') {
-    getInstance().track('success-rate', name, {value: 1}, plugin);
+    getLogger().track('success-rate', name, {value: 1}, plugin);
   } else if (result.kind === 'cancelled') {
-    getInstance().track('operation-cancelled', name, undefined, plugin);
+    getLogger().track('operation-cancelled', name, undefined, plugin);
   } else {
-    getInstance().track(
+    getLogger().track(
       'success-rate',
       name,
       {

desktop/flipper-common/package.json

@@ -9,7 +9,9 @@
   "license": "MIT",
   "bugs": "https://github.com/facebook/flipper/issues",
   "dependencies": {},
-  "devDependencies": {},
+  "devDependencies": {
+    "flipper-plugin-lib": "0.0.0"
+  },
   "peerDependencies": {},
   "scripts": {
     "reset": "rimraf lib *.tsbuildinfo",

desktop/flipper-common/src/fb-interfaces/Logger.tsx

@@ -1,44 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @format
- */
-
-export type LoggerTypes = 'error' | 'warn' | 'info' | 'debug';
-
-export type LoggerTrackType =
-  | 'duration'
-  | 'usage'
-  | 'performance'
-  | 'success-rate'
-  | 'operation-cancelled';
-
-export type LoggerArgs = {
-  isTest?: boolean;
-};
-
-export interface Logger {
-  track(
-    type: LoggerTrackType,
-    event: string,
-    data?: any,
-    plugin?: string,
-  ): void;
-
-  trackTimeSince(
-    mark: string,
-    eventName?: string | null | undefined,
-    data?: any,
-  ): void;
-
-  info(data: any, category: string): void;
-
-  warn(data: any, category: string): void;
-
-  error(data: any, category: string): void;
-
-  debug(data: any, category: string): void;
-}

desktop/flipper-common/src/index.tsx

@@ -12,7 +12,11 @@ export {
   LoggerTrackType,
   LoggerTypes,
   LoggerArgs,
-} from './fb-interfaces/Logger';
+  getLogger,
+  setLoggerInstance,
+  NoopLogger,
+} from './utils/Logger';
 export * from './server-types';
 export {sleep} from './utils/sleep';
 export {timeout} from './utils/timeout';
+export {isTest} from './utils/isTest';

desktop/flipper-common/src/utils/Logger.tsx

@@ -0,0 +1,95 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+import {isTest} from '../utils/isTest';
+
+export type LoggerTypes = 'error' | 'warn' | 'info' | 'debug';
+
+export type LoggerTrackType =
+  | 'duration'
+  | 'usage'
+  | 'performance'
+  | 'success-rate'
+  | 'operation-cancelled';
+
+export type LoggerArgs = {
+  isTest?: boolean;
+};
+
+export interface Logger {
+  track(
+    type: LoggerTrackType,
+    event: string,
+    data?: any,
+    plugin?: string,
+  ): void;
+
+  trackTimeSince(
+    mark: string,
+    eventName?: string | null | undefined,
+    data?: any,
+  ): void;
+
+  info(data: any, category: string): void;
+
+  warn(data: any, category: string): void;
+
+  error(data: any, category: string): void;
+
+  debug(data: any, category: string): void;
+}
+
+let instance: Logger | null = null;
+
+export function getLogger(): Logger {
+  if (instance) {
+    return instance;
+  }
+
+  if (isTest()) {
+    instance = new NoopLogger();
+    return instance;
+  }
+
+  throw new Error(
+    'Requested Logger instance without initializing it. Make sure init() is called at app start',
+  );
+}
+
+// only for testing
+export function setLoggerInstance(logger: Logger) {
+  if (!isTest() && instance) {
+    console.warn('Logger was already initialised');
+  }
+  instance = logger;
+}
+
+/**
+ * A logger that doesn't log anything, used in tests and public Flipper builds
+ */
+export class NoopLogger implements Logger {
+  constructor() {}
+
+  track(
+    _type: LoggerTrackType,
+    _event: string,
+    _data?: any,
+    _plugin?: string,
+  ) {}
+
+  trackTimeSince(_mark: string, _eventName?: string) {}
+
+  info(_data: any, _category: string) {}
+
+  warn(_data: any, _category: string) {}
+
+  error(_data: any, _category: string) {}
+
+  debug(_data: any, _category: string) {}
+}

desktop/flipper-common/src/utils/isTest.tsx

@@ -0,0 +1,14 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @format
+ */
+
+const _isTest = !!process.env.JEST_WORKER_ID;
+
+export function isTest(): boolean {
+  return _isTest;
+}

desktop/tsc-root/tsconfig.json

@@ -1,28 +1,46 @@
 {
-    "references": [
-        {
-            "path": "../babel-transformer"
-        },
-        {
-            "path": "../plugin-lib"
-        },
-        {
-            "path": "../flipper-plugin"
-        },
-        {
-            "path": "../pkg"
-        },
-        {
-            "path": "../pkg-lib"
-        },
-        {
-            "path": "../doctor"
-        },
-        {
-            "path": "../eslint-plugin-flipper"
-        },
-        {
-            "path": "../test-utils"
-        }
-    ]
+  "references": [
+    {
+      "path": "../babel-transformer"
+    },
+    {
+      "path": "../plugin-lib"
+    },
+    {
+      "path": "../flipper-common"
+    },
+    {
+      "path": "../flipper-dump"
+    },
+    {
+      "path": "../flipper-plugin"
+    },
+    {
+      "path": "../flipper-server"
+    },
+    {
+      "path": "../flipper-server-core"
+    },
+    {
+      "path": "../flipper-ui-browser"
+    },
+    {
+      "path": "../flipper-ui-core"
+    },
+    {
+      "path": "../pkg"
+    },
+    {
+      "path": "../pkg-lib"
+    },
+    {
+      "path": "../doctor"
+    },
+    {
+      "path": "../eslint-plugin-flipper"
+    },
+    {
+      "path": "../test-utils"
+    }
+  ]
 }