From a6dad1763cd4702a24ed428b96ffbd8b50de0cce Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Tue, 10 Dec 2019 14:21:14 -0800
Subject: [PATCH 01/13] initial log work

---
 packages/analytics/src/helpers.ts         |   5 +-
 packages/analytics/src/logger.ts          |  20 ++++
 packages/app-types/index.d.ts             |  27 +++++
 packages/app/src/firebaseNamespaceCore.ts |   3 +
 packages/logger/index.ts                  |  10 +-
 packages/logger/src/logger.ts             | 139 ++++++++++++++++------
 6 files changed, 156 insertions(+), 48 deletions(-)
 create mode 100644 packages/analytics/src/logger.ts

diff --git a/packages/analytics/src/helpers.ts b/packages/analytics/src/helpers.ts
index e0583611434..8b1dced7185 100644
--- a/packages/analytics/src/helpers.ts
+++ b/packages/analytics/src/helpers.ts
@@ -31,6 +31,7 @@ import {
   GTAG_URL
 } from './constants';
 import { FirebaseInstallations } from '@firebase/installations-types';
+import { logger } from './logger';
 
 /**
  * Initialize the analytics instance in gtag.js by calling config command with fid.
@@ -146,7 +147,7 @@ function wrapGtag(
             gtagParams || {}
           )
         )
-        .catch(e => console.error(e));
+        .catch(e => logger.error(e));
     } else if (command === GtagCommand.CONFIG) {
       const initializationPromiseToWait =
         initializedIdPromisesMap[idOrNameOrParams as string] ||
@@ -155,7 +156,7 @@ function wrapGtag(
         .then(() => {
           gtagCore(GtagCommand.CONFIG, idOrNameOrParams as string, gtagParams);
         })
-        .catch(e => console.error(e));
+        .catch(e => logger.error(e));
     } else {
       // SET command.
       // Splitting calls for CONFIG and SET to make it clear which signature
diff --git a/packages/analytics/src/logger.ts b/packages/analytics/src/logger.ts
new file mode 100644
index 00000000000..245183d890b
--- /dev/null
+++ b/packages/analytics/src/logger.ts
@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright 2019 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Logger } from '@firebase/logger';
+
+export const logger = new Logger('@firebase/analytics');
diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts
index 207e2040e39..55f00c5c16b 100644
--- a/packages/app-types/index.d.ts
+++ b/packages/app-types/index.d.ts
@@ -1,3 +1,5 @@
+import { FirebaseService } from './private';
+
 /**
  * @license
  * Copyright 2017 Google Inc.
@@ -31,6 +33,25 @@ export interface FirebaseAppConfig {
   automaticDataCollectionEnabled?: boolean;
 }
 
+export type LogLevelString =
+  | 'debug'
+  | 'verbose'
+  | 'info'
+  | 'warn'
+  | 'error'
+  | 'silent';
+
+export interface LogOptions {
+  level: LogLevelString;
+}
+
+export type LogCallback = (callbackParams: {
+  level: LogLevelString;
+  message: string;
+  source: FirebaseService | FirebaseApp;
+  type: string;
+}) => void;
+
 export class FirebaseApp {
   /**
    * The (read-only) name (identifier) for this App. '[DEFAULT]' is the default
@@ -105,6 +126,12 @@ export interface FirebaseNamespace {
    */
   registerVersion(library: string, version: string, variant?: string): void;
 
+  // Sets log level for all Firebase components.
+  setLogLevel(logLevel: LogLevelString): void;
+
+  // Sets log handler for all Firebase components.
+  onLog(logCallback: LogCallback, options: LogOptions): void;
+
   // The current SDK version.
   SDK_VERSION: string;
 }
diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts
index f17ad16f6b7..43ea652170a 100644
--- a/packages/app/src/firebaseNamespaceCore.ts
+++ b/packages/app/src/firebaseNamespaceCore.ts
@@ -34,6 +34,7 @@ import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
 import { DEFAULT_ENTRY_NAME, PLATFORM_LOG_STRING } from './constants';
 import { version } from '../../firebase/package.json';
 import { logger } from './logger';
+import { addLogCallback, setLogLevel } from '@firebase/logger';
 import { Component, ComponentType, Name } from '@firebase/component';
 
 /**
@@ -60,6 +61,8 @@ export function createFirebaseNamespaceCore(
     // @ts-ignore
     app,
     registerVersion,
+    setLogLevel,
+    onLog: addLogCallback,
     // @ts-ignore
     apps: null,
     SDK_VERSION: version,
diff --git a/packages/logger/index.ts b/packages/logger/index.ts
index 7030f3bc850..eb1494dc65c 100644
--- a/packages/logger/index.ts
+++ b/packages/logger/index.ts
@@ -15,12 +15,4 @@
  * limitations under the License.
  */
 
-import { instances, LogLevel } from './src/logger';
-
-export function setLogLevel(level: LogLevel): void {
-  instances.forEach(inst => {
-    inst.logLevel = level;
-  });
-}
-
-export { Logger, LogLevel, LogHandler } from './src/logger';
+export { setLogLevel, Logger, LogLevel, LogHandler, addLogCallback } from './src/logger';
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index c32d35a2e47..602aa3624ab 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -15,6 +15,14 @@
  * limitations under the License.
  */
 
+import {
+  LogCallback,
+  LogLevelString,
+  FirebaseApp,
+  LogOptions
+} from '@firebase/app-types';
+import { FirebaseService } from '@firebase/app-types/private';
+
 /**
  * A container for all of the Logger instances
  */
@@ -40,6 +48,15 @@ export enum LogLevel {
   SILENT
 }
 
+const levelStringToEnum = {
+  'debug': LogLevel.DEBUG,
+  'verbose': LogLevel.VERBOSE,
+  'info': LogLevel.INFO,
+  'warn': LogLevel.WARN,
+  'error': LogLevel.ERROR,
+  'silent': LogLevel.SILENT
+};
+
 /**
  * The default log level
  */
@@ -53,45 +70,45 @@ const defaultLogLevel: LogLevel = LogLevel.INFO;
 export type LogHandler = (
   loggerInstance: Logger,
   logType: LogLevel,
+  message: string,
+  source?: FirebaseService | FirebaseApp,
   ...args: unknown[]
 ) => void;
 
+/**
+ * By default, `console.debug` is not displayed in the developer console (in
+ * chrome). To avoid forcing users to have to opt-in to these logs twice
+ * (i.e. once for firebase, and once in the console), we are sending `DEBUG`
+ * logs to the `console.log` function.
+ */
+const ConsoleMethod = {
+  [LogLevel.DEBUG]: 'log',
+  [LogLevel.VERBOSE]: 'log',
+  [LogLevel.INFO]: 'info',
+  [LogLevel.WARN]: 'warn',
+  [LogLevel.ERROR]: 'error'
+};
+
 /**
  * The default log handler will forward DEBUG, VERBOSE, INFO, WARN, and ERROR
  * messages on to their corresponding console counterparts (if the log method
  * is supported by the current log level)
  */
-const defaultLogHandler: LogHandler = (instance, logType, ...args): void => {
+const defaultLogHandler: LogHandler = (instance, logType, message): void => {
   if (logType < instance.logLevel) {
     return;
   }
   const now = new Date().toISOString();
-  switch (logType) {
-    /**
-     * By default, `console.debug` is not displayed in the developer console (in
-     * chrome). To avoid forcing users to have to opt-in to these logs twice
-     * (i.e. once for firebase, and once in the console), we are sending `DEBUG`
-     * logs to the `console.log` function.
-     */
-    case LogLevel.DEBUG:
-      console.log(`[${now}]  ${instance.name}:`, ...args);
-      break;
-    case LogLevel.VERBOSE:
-      console.log(`[${now}]  ${instance.name}:`, ...args);
-      break;
-    case LogLevel.INFO:
-      console.info(`[${now}]  ${instance.name}:`, ...args);
-      break;
-    case LogLevel.WARN:
-      console.warn(`[${now}]  ${instance.name}:`, ...args);
-      break;
-    case LogLevel.ERROR:
-      console.error(`[${now}]  ${instance.name}:`, ...args);
-      break;
-    default:
-      throw new Error(
-        `Attempted to log a message with an invalid logType (value: ${logType})`
-      );
+  const method = ConsoleMethod[logType as keyof typeof ConsoleMethod];
+  if (method) {
+    console[method as 'log' | 'info' | 'warn' | 'error'](
+      `[${now}]  ${instance.name}:`,
+      message
+    );
+  } else {
+    throw new Error(
+      `Attempted to log a message with an invalid logType (value: ${logType})`
+    );
   }
 };
 
@@ -141,19 +158,67 @@ export class Logger {
    * The functions below are all based on the `console` interface
    */
 
-  debug(...args: unknown[]): void {
-    this._logHandler(this, LogLevel.DEBUG, ...args);
+  debug(
+    message: string,
+    source?: FirebaseService | FirebaseApp
+  ): void {
+    this._logHandler(this, LogLevel.DEBUG, message, source);
+  }
+  log(
+    message: string,
+    source?: FirebaseService | FirebaseApp
+  ): void {
+    this._logHandler(this, LogLevel.VERBOSE, message, source);
   }
-  log(...args: unknown[]): void {
-    this._logHandler(this, LogLevel.VERBOSE, ...args);
+  info(
+    message: string,
+    source?: FirebaseService | FirebaseApp
+  ): void {
+    this._logHandler(this, LogLevel.INFO, message, source);
   }
-  info(...args: unknown[]): void {
-    this._logHandler(this, LogLevel.INFO, ...args);
+  warn(
+    message: string,
+    source?: FirebaseService | FirebaseApp
+  ): void {
+    this._logHandler(this, LogLevel.WARN, message, source);
   }
-  warn(...args: unknown[]): void {
-    this._logHandler(this, LogLevel.WARN, ...args);
+  error(
+    message: string,
+    source?: FirebaseService | FirebaseApp
+  ): void {
+    this._logHandler(this, LogLevel.ERROR, message, source);
   }
-  error(...args: unknown[]): void {
-    this._logHandler(this, LogLevel.ERROR, ...args);
+}
+
+export function setLogLevel(level: LogLevelString | LogLevel): void {
+  const newLevel = typeof level === 'string' ? levelStringToEnum[level] : level;
+  instances.forEach(inst => {
+    inst.logLevel = newLevel;
+  });
+}
+
+export function addLogCallback(logCallback: LogCallback, options: LogOptions) {
+  for (const index in instances) {
+    const instance = instances[index];
+    let threshhold = instance.logLevel;
+    if (options && options.level) {
+      threshhold = levelStringToEnum[options.level];
+    }
+    instance.logHandler = (
+      instance: Logger,
+      level: LogLevel,
+      message: string,
+      source?: FirebaseService | FirebaseApp
+    ) => {
+      if (level >= threshhold && message && source) {
+        logCallback({
+          level: LogLevel[level].toLowerCase() as LogLevelString,
+          message,
+          type: instance.name,
+          source
+        });
+      }
+      defaultLogHandler(instance, level, message);
+    };
   }
 }

From 6675b7cd2d3a7abbd40193a8dd277879791d5466 Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Tue, 10 Dec 2019 14:21:24 -0800
Subject: [PATCH 02/13] [AUTOMATED]: Prettier Code Styling

---
 packages/logger/index.ts      |  8 +++++++-
 packages/logger/src/logger.ts | 25 +++++--------------------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/packages/logger/index.ts b/packages/logger/index.ts
index eb1494dc65c..9f2930dd967 100644
--- a/packages/logger/index.ts
+++ b/packages/logger/index.ts
@@ -15,4 +15,10 @@
  * limitations under the License.
  */
 
-export { setLogLevel, Logger, LogLevel, LogHandler, addLogCallback } from './src/logger';
+export {
+  setLogLevel,
+  Logger,
+  LogLevel,
+  LogHandler,
+  addLogCallback
+} from './src/logger';
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index 602aa3624ab..c1b70a3d10a 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -158,34 +158,19 @@ export class Logger {
    * The functions below are all based on the `console` interface
    */
 
-  debug(
-    message: string,
-    source?: FirebaseService | FirebaseApp
-  ): void {
+  debug(message: string, source?: FirebaseService | FirebaseApp): void {
     this._logHandler(this, LogLevel.DEBUG, message, source);
   }
-  log(
-    message: string,
-    source?: FirebaseService | FirebaseApp
-  ): void {
+  log(message: string, source?: FirebaseService | FirebaseApp): void {
     this._logHandler(this, LogLevel.VERBOSE, message, source);
   }
-  info(
-    message: string,
-    source?: FirebaseService | FirebaseApp
-  ): void {
+  info(message: string, source?: FirebaseService | FirebaseApp): void {
     this._logHandler(this, LogLevel.INFO, message, source);
   }
-  warn(
-    message: string,
-    source?: FirebaseService | FirebaseApp
-  ): void {
+  warn(message: string, source?: FirebaseService | FirebaseApp): void {
     this._logHandler(this, LogLevel.WARN, message, source);
   }
-  error(
-    message: string,
-    source?: FirebaseService | FirebaseApp
-  ): void {
+  error(message: string, source?: FirebaseService | FirebaseApp): void {
     this._logHandler(this, LogLevel.ERROR, message, source);
   }
 }

From 48913913e9e72341b95e1bf2d9db0892da1d1c9a Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 11 Dec 2019 14:09:22 -0800
Subject: [PATCH 03/13] Remove source for now

---
 packages/app-types/index.d.ts             |  4 +-
 packages/app/src/firebaseNamespaceCore.ts |  4 +-
 packages/logger/index.ts                  |  2 +-
 packages/logger/src/logger.ts             | 96 ++++++++++++++---------
 4 files changed, 62 insertions(+), 44 deletions(-)

diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts
index 55f00c5c16b..6695b0d772a 100644
--- a/packages/app-types/index.d.ts
+++ b/packages/app-types/index.d.ts
@@ -1,5 +1,3 @@
-import { FirebaseService } from './private';
-
 /**
  * @license
  * Copyright 2017 Google Inc.
@@ -48,7 +46,7 @@ export interface LogOptions {
 export type LogCallback = (callbackParams: {
   level: LogLevelString;
   message: string;
-  source: FirebaseService | FirebaseApp;
+  args: unknown[];
   type: string;
 }) => void;
 
diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts
index 43ea652170a..64f024fdd00 100644
--- a/packages/app/src/firebaseNamespaceCore.ts
+++ b/packages/app/src/firebaseNamespaceCore.ts
@@ -34,7 +34,7 @@ import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
 import { DEFAULT_ENTRY_NAME, PLATFORM_LOG_STRING } from './constants';
 import { version } from '../../firebase/package.json';
 import { logger } from './logger';
-import { addLogCallback, setLogLevel } from '@firebase/logger';
+import { setUserLogHandler, setLogLevel } from '@firebase/logger';
 import { Component, ComponentType, Name } from '@firebase/component';
 
 /**
@@ -62,7 +62,7 @@ export function createFirebaseNamespaceCore(
     app,
     registerVersion,
     setLogLevel,
-    onLog: addLogCallback,
+    onLog: setUserLogHandler,
     // @ts-ignore
     apps: null,
     SDK_VERSION: version,
diff --git a/packages/logger/index.ts b/packages/logger/index.ts
index 9f2930dd967..a280e5c217a 100644
--- a/packages/logger/index.ts
+++ b/packages/logger/index.ts
@@ -20,5 +20,5 @@ export {
   Logger,
   LogLevel,
   LogHandler,
-  addLogCallback
+  setUserLogHandler
 } from './src/logger';
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index c1b70a3d10a..8d2e5814cc6 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -15,13 +15,7 @@
  * limitations under the License.
  */
 
-import {
-  LogCallback,
-  LogLevelString,
-  FirebaseApp,
-  LogOptions
-} from '@firebase/app-types';
-import { FirebaseService } from '@firebase/app-types/private';
+import { LogCallback, LogLevelString, LogOptions } from '@firebase/app-types';
 
 /**
  * A container for all of the Logger instances
@@ -48,7 +42,7 @@ export enum LogLevel {
   SILENT
 }
 
-const levelStringToEnum = {
+const levelStringToEnum: { [key in LogLevelString]: LogLevel } = {
   'debug': LogLevel.DEBUG,
   'verbose': LogLevel.VERBOSE,
   'info': LogLevel.INFO,
@@ -70,8 +64,6 @@ const defaultLogLevel: LogLevel = LogLevel.INFO;
 export type LogHandler = (
   loggerInstance: Logger,
   logType: LogLevel,
-  message: string,
-  source?: FirebaseService | FirebaseApp,
   ...args: unknown[]
 ) => void;
 
@@ -141,7 +133,8 @@ export class Logger {
   }
 
   /**
-   * The log handler for the Logger instance.
+   * The main (internal) log handler for the Logger instance.
+   * Can be set to a new function in internal package code but not by user.
    */
   private _logHandler: LogHandler = defaultLogHandler;
   get logHandler(): LogHandler {
@@ -154,24 +147,41 @@ export class Logger {
     this._logHandler = val;
   }
 
+  /**
+   * The optional, additional, user-defined log handler for the Logger instance.
+   */
+  private _userLogHandler: LogHandler | null = null;
+  get userLogHandler(): LogHandler | null {
+    return this._userLogHandler;
+  }
+  set userLogHandler(val: LogHandler | null) {
+    this._userLogHandler = val;
+  }
+
   /**
    * The functions below are all based on the `console` interface
    */
 
-  debug(message: string, source?: FirebaseService | FirebaseApp): void {
-    this._logHandler(this, LogLevel.DEBUG, message, source);
+  debug(...args: unknown[]): void {
+    this._userLogHandler && this._userLogHandler(this, LogLevel.DEBUG, ...args);
+    this._logHandler(this, LogLevel.DEBUG, ...args);
   }
-  log(message: string, source?: FirebaseService | FirebaseApp): void {
-    this._logHandler(this, LogLevel.VERBOSE, message, source);
+  log(...args: unknown[]): void {
+    this._userLogHandler &&
+      this._userLogHandler(this, LogLevel.VERBOSE, ...args);
+    this._logHandler(this, LogLevel.VERBOSE, ...args);
   }
-  info(message: string, source?: FirebaseService | FirebaseApp): void {
-    this._logHandler(this, LogLevel.INFO, message, source);
+  info(...args: unknown[]): void {
+    this._userLogHandler && this._userLogHandler(this, LogLevel.INFO, ...args);
+    this._logHandler(this, LogLevel.INFO, ...args);
   }
-  warn(message: string, source?: FirebaseService | FirebaseApp): void {
-    this._logHandler(this, LogLevel.WARN, message, source);
+  warn(...args: unknown[]): void {
+    this._userLogHandler && this._userLogHandler(this, LogLevel.WARN, ...args);
+    this._logHandler(this, LogLevel.WARN, ...args);
   }
-  error(message: string, source?: FirebaseService | FirebaseApp): void {
-    this._logHandler(this, LogLevel.ERROR, message, source);
+  error(...args: unknown[]): void {
+    this._userLogHandler && this._userLogHandler(this, LogLevel.ERROR, ...args);
+    this._logHandler(this, LogLevel.ERROR, ...args);
   }
 }
 
@@ -182,28 +192,38 @@ export function setLogLevel(level: LogLevelString | LogLevel): void {
   });
 }
 
-export function addLogCallback(logCallback: LogCallback, options: LogOptions) {
+export function setUserLogHandler(
+  logCallback: LogCallback | null,
+  options: LogOptions
+) {
+  if (typeof logCallback !== 'function') {
+    console.warn('First argument to `onLog` must be a function.');
+    return;
+  }
   for (const index in instances) {
     const instance = instances[index];
     let threshhold = instance.logLevel;
     if (options && options.level) {
       threshhold = levelStringToEnum[options.level];
     }
-    instance.logHandler = (
-      instance: Logger,
-      level: LogLevel,
-      message: string,
-      source?: FirebaseService | FirebaseApp
-    ) => {
-      if (level >= threshhold && message && source) {
-        logCallback({
-          level: LogLevel[level].toLowerCase() as LogLevelString,
-          message,
-          type: instance.name,
-          source
-        });
-      }
-      defaultLogHandler(instance, level, message);
-    };
+    if (logCallback === null) {
+      instance.userLogHandler = null;
+    } else {
+      instance.userLogHandler = (
+        instance: Logger,
+        level: LogLevel,
+        ...args: unknown[]
+      ) => {
+        const message = args.map(arg => (arg as object).toString()).join(' ');
+        if (level >= threshhold) {
+          logCallback({
+            level: LogLevel[level].toLowerCase() as LogLevelString,
+            message,
+            args,
+            type: instance.name
+          });
+        }
+      };
+    }
   }
 }

From fd3fef0a0312de80332d18f126d018e4495f61fd Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 11 Dec 2019 14:36:35 -0800
Subject: [PATCH 04/13] Fix some lint issues

---
 packages/logger/src/logger.ts | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index 8d2e5814cc6..2e4125da15d 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -195,13 +195,12 @@ export function setLogLevel(level: LogLevelString | LogLevel): void {
 export function setUserLogHandler(
   logCallback: LogCallback | null,
   options: LogOptions
-) {
+): void {
   if (typeof logCallback !== 'function') {
     console.warn('First argument to `onLog` must be a function.');
     return;
   }
-  for (const index in instances) {
-    const instance = instances[index];
+  for (const instance of instances) {
     let threshhold = instance.logLevel;
     if (options && options.level) {
       threshhold = levelStringToEnum[options.level];

From 92ec763f084a3a691f3cbbd50d41745ad5330f94 Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 11 Dec 2019 15:13:02 -0800
Subject: [PATCH 05/13] Add logger dep

---
 packages/analytics/package.json | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/analytics/package.json b/packages/analytics/package.json
index b620fbe6840..05982187fe8 100644
--- a/packages/analytics/package.json
+++ b/packages/analytics/package.json
@@ -26,6 +26,7 @@
   "dependencies": {
     "@firebase/analytics-types": "0.2.7",
     "@firebase/installations": "0.4.4",
+    "@firebase/logger": "0.1.36",
     "@firebase/util": "0.2.41",
     "@firebase/component": "0.1.6",
     "tslib": "1.11.1"

From ae1f8a7277d22fb0b4932e574765573b5cbdf55e Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Mon, 23 Dec 2019 11:46:20 -0800
Subject: [PATCH 06/13] Refactor types, add doc comments.

---
 packages/app-types/index.d.ts | 20 +-------------
 packages/firebase/index.d.ts  | 52 +++++++++++++++++++++++++++++++++++
 packages/logger/index.ts      |  5 +++-
 packages/logger/src/logger.ts | 44 +++++++++++++++++++++++++----
 4 files changed, 96 insertions(+), 25 deletions(-)

diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts
index 6695b0d772a..0b0bd1dc075 100644
--- a/packages/app-types/index.d.ts
+++ b/packages/app-types/index.d.ts
@@ -14,6 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import { LogCallback, LogLevelString, LogOptions } from '@firebase/logger';
 
 export type FirebaseOptions = {
   apiKey?: string;
@@ -31,25 +32,6 @@ export interface FirebaseAppConfig {
   automaticDataCollectionEnabled?: boolean;
 }
 
-export type LogLevelString =
-  | 'debug'
-  | 'verbose'
-  | 'info'
-  | 'warn'
-  | 'error'
-  | 'silent';
-
-export interface LogOptions {
-  level: LogLevelString;
-}
-
-export type LogCallback = (callbackParams: {
-  level: LogLevelString;
-  message: string;
-  args: unknown[];
-  type: string;
-}) => void;
-
 export class FirebaseApp {
   /**
    * The (read-only) name (identifier) for this App. '[DEFAULT]' is the default
diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts
index 037a2cdc631..f47f0e3d017 100644
--- a/packages/firebase/index.d.ts
+++ b/packages/firebase/index.d.ts
@@ -78,6 +78,15 @@ declare namespace firebase {
     complete: CompleteFn;
   }
 
+  /**
+   * The JS SDK supports 5 log levels and also allows a user the ability to
+   * silence the logs altogether.
+   *
+   * The order is as follows:
+   * debug < verbose < info < warn < error
+   */
+  type LogLevel = 'debug' | 'verbose' | 'info' | 'warn' | 'error' | 'silent';
+
   /**
    * The current SDK version.
    */
@@ -95,6 +104,49 @@ declare namespace firebase {
     variant?: string
   ): void;
 
+  /**
+   * Sets log level for all Firebase packages.
+   *
+   * All of the log types above the current log level will be captured (i.e. if
+   * you set the log level to `info`, errors will still be logged, but `debug` and
+   * `verbose` logs will not)
+   */
+  function setLogLevel(logLevel: LogLevel): void;
+
+  /**
+   * Sets log handler for all Firebase packages.
+   * @param logCallback An optional custom log handler that will execute user code whenever
+   * the Firebase SDK makes a logging call.
+   */
+  function onLog(
+    logCallback: (callbackParams: {
+      /**
+       * Level of event logged.
+       */
+      level: LogLevel;
+      /**
+       * Any text from logged arguments joined into one string.
+       */
+      message: string;
+      /**
+       * The raw arguments passed to the log call.
+       */
+      args: unknown[];
+      /**
+       * A string indicating the source of the log call, usually a package name
+       * such as `@firebase/firestore`.
+       */
+      type: string;
+    }) => void,
+    options: {
+      /**
+       * Threshhold log level. Only logs at or above this level will trigger the `logCallback`
+       * passed to `onLog`.
+       */
+      level: LogLevel;
+    }
+  ): void;
+
   /**
    * @hidden
    */
diff --git a/packages/logger/index.ts b/packages/logger/index.ts
index a280e5c217a..39bfbcf7272 100644
--- a/packages/logger/index.ts
+++ b/packages/logger/index.ts
@@ -20,5 +20,8 @@ export {
   Logger,
   LogLevel,
   LogHandler,
-  setUserLogHandler
+  setUserLogHandler,
+  LogCallback,
+  LogLevelString,
+  LogOptions
 } from './src/logger';
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index 2e4125da15d..777c503251a 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -15,7 +15,24 @@
  * limitations under the License.
  */
 
-import { LogCallback, LogLevelString, LogOptions } from '@firebase/app-types';
+export type LogLevelString =
+  | 'debug'
+  | 'verbose'
+  | 'info'
+  | 'warn'
+  | 'error'
+  | 'silent';
+
+export interface LogOptions {
+  level: LogLevelString;
+}
+
+export type LogCallback = (callbackParams: {
+  level: LogLevelString;
+  message: string;
+  args: unknown[];
+  type: string;
+}) => void;
 
 /**
  * A container for all of the Logger instances
@@ -196,9 +213,8 @@ export function setUserLogHandler(
   logCallback: LogCallback | null,
   options: LogOptions
 ): void {
-  if (typeof logCallback !== 'function') {
-    console.warn('First argument to `onLog` must be a function.');
-    return;
+  if (logCallback !== null && typeof logCallback !== 'function') {
+    throw new TypeError('First argument to `onLog` must be null or a function.');
   }
   for (const instance of instances) {
     let threshhold = instance.logLevel;
@@ -213,7 +229,25 @@ export function setUserLogHandler(
         level: LogLevel,
         ...args: unknown[]
       ) => {
-        const message = args.map(arg => (arg as object).toString()).join(' ');
+        const message = args.map(arg => {
+          if (arg == null) {
+            return null;
+          } else if (typeof arg === 'string') {
+            return arg;
+          } else if (typeof arg === 'number' || typeof arg === 'boolean') {
+            return arg.toString();
+          } else if (arg instanceof Error) {
+            return arg.message;
+          } else {
+            try {
+              return JSON.stringify(arg);
+            } catch(ignored) {
+              return null;
+            }
+          }
+        })
+        .filter(arg => arg)
+        .join(' ');
         if (level >= threshhold) {
           logCallback({
             level: LogLevel[level].toLowerCase() as LogLevelString,

From 5cb934afd9f32f370fc98be9e3906adbfa871a9f Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Mon, 23 Dec 2019 11:47:11 -0800
Subject: [PATCH 07/13] [AUTOMATED]: Prettier Code Styling

---
 packages/logger/src/logger.ts | 39 +++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index 777c503251a..99db5d674e6 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -214,7 +214,9 @@ export function setUserLogHandler(
   options: LogOptions
 ): void {
   if (logCallback !== null && typeof logCallback !== 'function') {
-    throw new TypeError('First argument to `onLog` must be null or a function.');
+    throw new TypeError(
+      'First argument to `onLog` must be null or a function.'
+    );
   }
   for (const instance of instances) {
     let threshhold = instance.logLevel;
@@ -229,25 +231,26 @@ export function setUserLogHandler(
         level: LogLevel,
         ...args: unknown[]
       ) => {
-        const message = args.map(arg => {
-          if (arg == null) {
-            return null;
-          } else if (typeof arg === 'string') {
-            return arg;
-          } else if (typeof arg === 'number' || typeof arg === 'boolean') {
-            return arg.toString();
-          } else if (arg instanceof Error) {
-            return arg.message;
-          } else {
-            try {
-              return JSON.stringify(arg);
-            } catch(ignored) {
+        const message = args
+          .map(arg => {
+            if (arg == null) {
               return null;
+            } else if (typeof arg === 'string') {
+              return arg;
+            } else if (typeof arg === 'number' || typeof arg === 'boolean') {
+              return arg.toString();
+            } else if (arg instanceof Error) {
+              return arg.message;
+            } else {
+              try {
+                return JSON.stringify(arg);
+              } catch (ignored) {
+                return null;
+              }
             }
-          }
-        })
-        .filter(arg => arg)
-        .join(' ');
+          })
+          .filter(arg => arg)
+          .join(' ');
         if (level >= threshhold) {
           logCallback({
             level: LogLevel[level].toLowerCase() as LogLevelString,

From 43222eb821268dfa7ab05aea6cf1111151067981 Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 12 Feb 2020 11:12:13 -0800
Subject: [PATCH 08/13] Add tests, fix types

---
 packages/app-types/index.d.ts              |   2 +-
 packages/app/test/clientLogger.test.ts     | 105 +++++++++++
 packages/firebase/index.d.ts               |   2 +-
 packages/logger/karma.conf.js              |   4 +-
 packages/logger/src/logger.ts              |   8 +-
 packages/logger/test/custom-logger.test.ts | 204 +++++++++++++++++++++
 packages/logger/test/logger.test.ts        |  13 +-
 7 files changed, 329 insertions(+), 9 deletions(-)
 create mode 100644 packages/app/test/clientLogger.test.ts
 create mode 100644 packages/logger/test/custom-logger.test.ts

diff --git a/packages/app-types/index.d.ts b/packages/app-types/index.d.ts
index 0b0bd1dc075..fd442d76343 100644
--- a/packages/app-types/index.d.ts
+++ b/packages/app-types/index.d.ts
@@ -110,7 +110,7 @@ export interface FirebaseNamespace {
   setLogLevel(logLevel: LogLevelString): void;
 
   // Sets log handler for all Firebase components.
-  onLog(logCallback: LogCallback, options: LogOptions): void;
+  onLog(logCallback: LogCallback, options?: LogOptions): void;
 
   // The current SDK version.
   SDK_VERSION: string;
diff --git a/packages/app/test/clientLogger.test.ts b/packages/app/test/clientLogger.test.ts
new file mode 100644
index 00000000000..17e20fe45e7
--- /dev/null
+++ b/packages/app/test/clientLogger.test.ts
@@ -0,0 +1,105 @@
+/**
+ * @license
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { FirebaseNamespace, VersionService } from '@firebase/app-types';
+import { _FirebaseApp, _FirebaseNamespace } from '@firebase/app-types/private';
+import { createFirebaseNamespace } from '../src/firebaseNamespace';
+import { expect } from 'chai';
+import { spy as Spy } from 'sinon';
+import './setup';
+import { Logger } from '@firebase/logger';
+import { registerCoreComponents } from '../src/registerCoreComponents';
+import {
+  Component,
+  ComponentType,
+  ComponentContainer
+} from '@firebase/component';
+
+declare module '@firebase/component' {
+  interface NameServiceMapping {
+    'vs1': VersionService;
+    'vs2': VersionService;
+    'test-shell': Promise<void>;
+  }
+}
+
+describe('User Log Methods', () => {
+  describe('Integration Tests', () => {
+    let firebase: FirebaseNamespace;
+    let result: any = null;
+    let warnSpy = Spy(console, 'warn');
+    let infoSpy = Spy(console, 'info');
+    let logSpy = Spy(console, 'log');
+
+    beforeEach(() => {
+      firebase = createFirebaseNamespace();
+    });
+
+    it(`respects log level set through firebase.setLogLevel()`, () => {
+      firebase.initializeApp({});
+      registerCoreComponents(firebase);
+      (firebase as _FirebaseNamespace).INTERNAL.registerComponent(
+        new Component(
+          'test-shell',
+          async (container: ComponentContainer) => {
+
+            const logger = new Logger('@firebase/logger-test');
+            logger.warn('hello');
+            expect(warnSpy.called).to.be.true;
+            (firebase as _FirebaseNamespace).setLogLevel('warn');
+            logger.info('hi');
+            expect(infoSpy.called).to.be.false;
+            logger.log('hi');
+            expect(logSpy.called).to.be.false;
+            logSpy.resetHistory();
+            infoSpy.resetHistory();
+            (firebase as _FirebaseNamespace).setLogLevel('debug');
+            logger.info('hi');
+            expect(infoSpy.called).to.be.true;
+            logger.log('hi');
+            expect(logSpy.called).to.be.true;
+          },
+          ComponentType.PUBLIC
+        )
+      );
+      return (firebase as any)['test-shell']();
+    });
+
+    it(`correctly triggers callback given to firebase.onLog()`, () => {
+      firebase.initializeApp({});
+      registerCoreComponents(firebase);
+      (firebase as _FirebaseNamespace).INTERNAL.registerComponent(
+        new Component(
+          'test-shell',
+          async (container: ComponentContainer) => {
+
+            const logger = new Logger('@firebase/logger-test');
+            (firebase as _FirebaseNamespace).onLog((logData) => {
+              result = logData;
+            });
+            logger.info('hi');
+            expect(result.level).to.equal('info');
+            expect(result.message).to.equal('hi');
+            expect(infoSpy.called).to.be.true;
+          },
+          ComponentType.PUBLIC
+        )
+      );
+      return (firebase as any)['test-shell']();
+    });
+  });
+});
diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts
index f47f0e3d017..f016f8e200e 100644
--- a/packages/firebase/index.d.ts
+++ b/packages/firebase/index.d.ts
@@ -138,7 +138,7 @@ declare namespace firebase {
        */
       type: string;
     }) => void,
-    options: {
+    options?: {
       /**
        * Threshhold log level. Only logs at or above this level will trigger the `logCallback`
        * passed to `onLog`.
diff --git a/packages/logger/karma.conf.js b/packages/logger/karma.conf.js
index c422d2666ef..3b5d500ceb5 100644
--- a/packages/logger/karma.conf.js
+++ b/packages/logger/karma.conf.js
@@ -15,8 +15,6 @@
  * limitations under the License.
  */
 
-const karma = require('karma');
-const path = require('path');
 const karmaBase = require('../../config/karma.base');
 
 const files = [`test/**/*`];
@@ -24,7 +22,7 @@ const files = [`test/**/*`];
 module.exports = function(config) {
   const karmaConfig = Object.assign({}, karmaBase, {
     // files to load into karma
-    files: files,
+    files,
     // frameworks to use
     // available frameworks: https://npmjs.org/browse/keyword/karma-adapter
     frameworks: ['mocha']
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index 99db5d674e6..a1839d7d0a9 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -27,12 +27,14 @@ export interface LogOptions {
   level: LogLevelString;
 }
 
-export type LogCallback = (callbackParams: {
+export type LogCallback = (callbackParams: LogCallbackParams) => void;
+
+export interface LogCallbackParams {
   level: LogLevelString;
   message: string;
   args: unknown[];
   type: string;
-}) => void;
+};
 
 /**
  * A container for all of the Logger instances
@@ -211,7 +213,7 @@ export function setLogLevel(level: LogLevelString | LogLevel): void {
 
 export function setUserLogHandler(
   logCallback: LogCallback | null,
-  options: LogOptions
+  options?: LogOptions
 ): void {
   if (logCallback !== null && typeof logCallback !== 'function') {
     throw new TypeError(
diff --git a/packages/logger/test/custom-logger.test.ts b/packages/logger/test/custom-logger.test.ts
new file mode 100644
index 00000000000..167b581b922
--- /dev/null
+++ b/packages/logger/test/custom-logger.test.ts
@@ -0,0 +1,204 @@
+/**
+ * @license
+ * Copyright 2018 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect } from 'chai';
+import { spy } from 'sinon';
+import { Logger, setLogLevel } from '../src/logger';
+import { setUserLogHandler } from '../index';
+
+describe(`Custom log handler`, () => {
+  const client1 = new Logger('@firebase/test-logger');
+  const client2 = new Logger('@firebase/other-logger');
+  let result: any;
+  let spies: any = {};
+
+  describe('Callback log level set to INFO (default)', () => {
+    before(() => {
+      setUserLogHandler((callbackParams) => {
+        result = callbackParams;
+      });
+    });
+    
+    beforeEach(() => {
+      result = null;
+      spies = {
+        logSpy: spy(console, 'log'),
+        infoSpy: spy(console, 'info'),
+        warnSpy: spy(console, 'warn'),
+        errorSpy: spy(console, 'error')
+      };
+    });
+
+    afterEach(() => {
+      spies.logSpy.restore();
+      spies.infoSpy.restore();
+      spies.warnSpy.restore();
+      spies.errorSpy.restore();
+    });
+
+    it('calls custom callback with correct data for calling instance', () => {
+      client1.info('info message!');
+      expect(result.message).to.equal('info message!');
+      expect(result.args[0]).to.equal('info message!');
+      expect(result.level).to.equal('info');
+      expect(result.type).to.equal('@firebase/test-logger');
+      expect(spies.infoSpy.called).to.be.true;
+      spies.infoSpy.resetHistory();
+      client2.info('another info message!');
+      expect(result.message).to.equal('another info message!');
+      expect(result.args[0]).to.equal('another info message!');
+      expect(result.level).to.equal('info');
+      expect(result.type).to.equal('@firebase/other-logger');
+      expect(spies.infoSpy.called).to.be.true;
+    });
+
+    it('parses multiple arguments correctly', () => {
+      client1.info('info message!', ['hello'], 1, { a: 3 });
+      expect(result.message).to.equal('info message! ["hello"] 1 {"a":3}');
+      expect(result.args).to.deep.equal(['info message!', ['hello'], 1, { a: 3 }]);
+      expect(result.level).to.equal('info');
+      expect(result.type).to.equal('@firebase/test-logger');
+    });
+
+    it('calls custom callback when log call is above set log level', () => {
+      // Info was already tested above.
+      client1.warn('warning message!');
+      expect(result.message).to.equal('warning message!');
+      expect(result.level).to.equal('warn');
+      expect(spies.warnSpy.called).to.be.true;
+      client1.error('error message!');
+      expect(result.message).to.equal('error message!');
+      expect(result.level).to.equal('error');
+      expect(spies.errorSpy.called).to.be.true;
+    });
+  
+    it('does not call custom callback when log call is not above set log level', () => {
+      client1.log('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.logSpy.called).to.be.false;
+      client1.debug('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.logSpy.called).to.be.false;
+    });
+  });
+
+  describe('Callback log level set to WARN with options', () => {
+    before(() => {
+      setUserLogHandler((callbackParams) => {
+        result = callbackParams;
+      }, { level: 'warn' });
+    });
+    
+    beforeEach(() => {
+      result = null;
+      spies = {
+        logSpy: spy(console, 'log'),
+        infoSpy: spy(console, 'info'),
+        warnSpy: spy(console, 'warn'),
+        errorSpy: spy(console, 'error')
+      };
+    });
+
+    afterEach(() => {
+      spies.logSpy.restore();
+      spies.infoSpy.restore();
+      spies.warnSpy.restore();
+      spies.errorSpy.restore();
+    });
+
+    it('calls custom callback when log call is above set log level', () => {
+      client1.warn('warning message!');
+      expect(result.message).to.equal('warning message!');
+      expect(result.args[0]).to.equal('warning message!');
+      expect(result.level).to.equal('warn');
+      expect(result.type).to.equal('@firebase/test-logger');
+      expect(spies.warnSpy.called).to.be.true;
+      client1.error('error message!');
+      expect(result.message).to.equal('error message!');
+      expect(result.level).to.equal('error');
+      expect(spies.errorSpy.called).to.be.true;
+    });
+  
+    it('does not call custom callback when log call is not above set log level', () => {
+      client1.debug('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.logSpy.called).to.be.false;
+      client1.log('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.logSpy.called).to.be.false;
+      client1.info('message you should not see');
+      expect(result).to.be.null;
+    });
+
+    it('logLevel set in setUserLogHandler should not affect internal logging level', () => {
+      client1.info('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.infoSpy.called).to.be.true;
+    });
+  });
+
+  describe('Global log level set to VERBOSE with setLogLevel()', () => {
+    before(() => {
+      setLogLevel('verbose');
+      setUserLogHandler((callbackParams) => {
+        result = callbackParams;
+      });
+    });
+    
+    beforeEach(() => {
+      result = null;
+      spies = {
+        logSpy: spy(console, 'log'),
+        infoSpy: spy(console, 'info'),
+        warnSpy: spy(console, 'warn'),
+        errorSpy: spy(console, 'error')
+      };
+    });
+
+    afterEach(() => {
+      spies.logSpy.restore();
+      spies.infoSpy.restore();
+      spies.warnSpy.restore();
+      spies.errorSpy.restore();
+    });
+
+    it('calls custom callback when log call is above set log level', () => {
+      client1.log('log message!');
+      expect(result.message).to.equal('log message!');
+      expect(result.level).to.equal('verbose');
+      expect(spies.logSpy.called).to.be.true;
+      client1.info('info message!');
+      expect(result.message).to.equal('info message!');
+      expect(result.level).to.equal('info');
+      expect(spies.infoSpy.called).to.be.true;
+      client1.warn('warning message!');
+      expect(result.message).to.equal('warning message!');
+      expect(result.level).to.equal('warn');
+      expect(spies.warnSpy.called).to.be.true;
+      client1.error('error message!');
+      expect(result.message).to.equal('error message!');
+      expect(result.level).to.equal('error');
+      expect(spies.errorSpy.called).to.be.true;
+    });
+  
+    it('does not call custom callback when log call is not above set log level', () => {
+      client1.debug('message you should not see');
+      expect(result).to.be.null;
+      expect(spies.logSpy.called).to.be.false;
+    });
+  });
+});
diff --git a/packages/logger/test/logger.test.ts b/packages/logger/test/logger.test.ts
index a0020599ad0..d994fce7561 100644
--- a/packages/logger/test/logger.test.ts
+++ b/packages/logger/test/logger.test.ts
@@ -77,7 +77,18 @@ describe('@firebase/logger', () => {
     testLog(message, 'error', true);
   });
 
-  describe('Defaults to LogLevel.NOTICE', () => {
+  describe('Can set log level with string', () => {
+    beforeEach(() => {
+      setLogLevel('warn');
+    });
+    testLog(message, 'debug', false);
+    testLog(message, 'log', false);
+    testLog(message, 'info', false);
+    testLog(message, 'warn', true);
+    testLog(message, 'error', true);
+  });
+
+  describe('Defaults to LogLevel.INFO', () => {
     testLog(message, 'debug', false);
     testLog(message, 'log', false);
     testLog(message, 'info', true);

From c81e8d892b063d48a63d99d5cf84719be393a8d6 Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 12 Feb 2020 11:36:02 -0800
Subject: [PATCH 09/13] Fix lint

---
 packages/app/test/clientLogger.test.ts | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/packages/app/test/clientLogger.test.ts b/packages/app/test/clientLogger.test.ts
index 17e20fe45e7..0d8db9f38e8 100644
--- a/packages/app/test/clientLogger.test.ts
+++ b/packages/app/test/clientLogger.test.ts
@@ -25,8 +25,7 @@ import { Logger } from '@firebase/logger';
 import { registerCoreComponents } from '../src/registerCoreComponents';
 import {
   Component,
-  ComponentType,
-  ComponentContainer
+  ComponentType
 } from '@firebase/component';
 
 declare module '@firebase/component' {
@@ -41,9 +40,9 @@ describe('User Log Methods', () => {
   describe('Integration Tests', () => {
     let firebase: FirebaseNamespace;
     let result: any = null;
-    let warnSpy = Spy(console, 'warn');
-    let infoSpy = Spy(console, 'info');
-    let logSpy = Spy(console, 'log');
+    const warnSpy = Spy(console, 'warn');
+    const infoSpy = Spy(console, 'info');
+    const logSpy = Spy(console, 'log');
 
     beforeEach(() => {
       firebase = createFirebaseNamespace();
@@ -55,8 +54,7 @@ describe('User Log Methods', () => {
       (firebase as _FirebaseNamespace).INTERNAL.registerComponent(
         new Component(
           'test-shell',
-          async (container: ComponentContainer) => {
-
+          async () => {
             const logger = new Logger('@firebase/logger-test');
             logger.warn('hello');
             expect(warnSpy.called).to.be.true;
@@ -85,8 +83,7 @@ describe('User Log Methods', () => {
       (firebase as _FirebaseNamespace).INTERNAL.registerComponent(
         new Component(
           'test-shell',
-          async (container: ComponentContainer) => {
-
+          async () => {
             const logger = new Logger('@firebase/logger-test');
             (firebase as _FirebaseNamespace).onLog((logData) => {
               result = logData;

From 312c43c76e8827515188aaa32732737e89c1ec0f Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Thu, 5 Mar 2020 12:28:11 -0800
Subject: [PATCH 10/13] Address documentation comments

---
 packages/firebase/index.d.ts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts
index f016f8e200e..998be90fbc3 100644
--- a/packages/firebase/index.d.ts
+++ b/packages/firebase/index.d.ts
@@ -83,7 +83,7 @@ declare namespace firebase {
    * silence the logs altogether.
    *
    * The order is as follows:
-   * debug < verbose < info < warn < error
+   * silent < debug < verbose < info < warn < error
    */
   type LogLevel = 'debug' | 'verbose' | 'info' | 'warn' | 'error' | 'silent';
 
@@ -107,15 +107,15 @@ declare namespace firebase {
   /**
    * Sets log level for all Firebase packages.
    *
-   * All of the log types above the current log level will be captured (i.e. if
-   * you set the log level to `info`, errors will still be logged, but `debug` and
-   * `verbose` logs will not)
+   * All of the log types above the current log level are captured (i.e. if
+   * you set the log level to `info`, errors are logged, but `debug` and
+   * `verbose` logs are not).
    */
   function setLogLevel(logLevel: LogLevel): void;
 
   /**
    * Sets log handler for all Firebase packages.
-   * @param logCallback An optional custom log handler that will execute user code whenever
+   * @param logCallback An optional custom log handler that executes user code whenever
    * the Firebase SDK makes a logging call.
    */
   function onLog(
@@ -140,7 +140,7 @@ declare namespace firebase {
     }) => void,
     options?: {
       /**
-       * Threshhold log level. Only logs at or above this level will trigger the `logCallback`
+       * Threshhold log level. Only logs at or above this level trigger the `logCallback`
        * passed to `onLog`.
        */
       level: LogLevel;

From 249d25610b54a4cf43c3cc625f445cbf5f2cca16 Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Thu, 5 Mar 2020 13:01:52 -0800
Subject: [PATCH 11/13] Address some PR comments

---
 packages/app/src/errors.ts                 |  6 ++++--
 packages/app/src/firebaseNamespaceCore.ts  | 11 +++++++++--
 packages/app/test/clientLogger.test.ts     |  3 +++
 packages/logger/src/logger.ts              |  9 ++-------
 packages/logger/test/custom-logger.test.ts |  6 +++++-
 5 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/packages/app/src/errors.ts b/packages/app/src/errors.ts
index e6685fdd210..0224b34c6ff 100644
--- a/packages/app/src/errors.ts
+++ b/packages/app/src/errors.ts
@@ -22,7 +22,8 @@ export const enum AppError {
   BAD_APP_NAME = 'bad-app-name',
   DUPLICATE_APP = 'duplicate-app',
   APP_DELETED = 'app-deleted',
-  INVALID_APP_ARGUMENT = 'invalid-app-argument'
+  INVALID_APP_ARGUMENT = 'invalid-app-argument',
+  INVALID_LOG_ARGUMENT = 'invalid-log-argument'
 }
 
 const ERRORS: ErrorMap<AppError> = {
@@ -34,7 +35,8 @@ const ERRORS: ErrorMap<AppError> = {
   [AppError.APP_DELETED]: "Firebase App named '{$appName}' already deleted",
   [AppError.INVALID_APP_ARGUMENT]:
     'firebase.{$appName}() takes either no argument or a ' +
-    'Firebase App instance.'
+    'Firebase App instance.',
+  [AppError.INVALID_LOG_ARGUMENT]: 'First argument to `onLog` must be null or a function.'
 };
 
 type ErrorParams = { [key in AppError]: { appName: string } };
diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts
index 64f024fdd00..b28b3880bb8 100644
--- a/packages/app/src/firebaseNamespaceCore.ts
+++ b/packages/app/src/firebaseNamespaceCore.ts
@@ -34,7 +34,7 @@ import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
 import { DEFAULT_ENTRY_NAME, PLATFORM_LOG_STRING } from './constants';
 import { version } from '../../firebase/package.json';
 import { logger } from './logger';
-import { setUserLogHandler, setLogLevel } from '@firebase/logger';
+import { setUserLogHandler, setLogLevel, LogCallback, LogOptions } from '@firebase/logger';
 import { Component, ComponentType, Name } from '@firebase/component';
 
 /**
@@ -62,7 +62,14 @@ export function createFirebaseNamespaceCore(
     app,
     registerVersion,
     setLogLevel,
-    onLog: setUserLogHandler,
+    onLog: (
+      logCallback: LogCallback | null,
+      options?: LogOptions) => {
+        if (logCallback !== null && typeof logCallback !== 'function') {
+          throw ERROR_FACTORY.create(AppError.INVALID_LOG_ARGUMENT, { appName: name });
+        }
+        setUserLogHandler(logCallback, options);
+      },
     // @ts-ignore
     apps: null,
     SDK_VERSION: version,
diff --git a/packages/app/test/clientLogger.test.ts b/packages/app/test/clientLogger.test.ts
index 0d8db9f38e8..06726fefd34 100644
--- a/packages/app/test/clientLogger.test.ts
+++ b/packages/app/test/clientLogger.test.ts
@@ -78,6 +78,7 @@ describe('User Log Methods', () => {
     });
 
     it(`correctly triggers callback given to firebase.onLog()`, () => {
+      // Note: default log level is INFO.
       firebase.initializeApp({});
       registerCoreComponents(firebase);
       (firebase as _FirebaseNamespace).INTERNAL.registerComponent(
@@ -91,6 +92,8 @@ describe('User Log Methods', () => {
             logger.info('hi');
             expect(result.level).to.equal('info');
             expect(result.message).to.equal('hi');
+            expect(result.args).to.deep.equal(['hi']);
+            expect(result.type).to.equal('@firebase/logger-test');
             expect(infoSpy.called).to.be.true;
           },
           ComponentType.PUBLIC
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index a1839d7d0a9..f83f390bf1e 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -105,7 +105,7 @@ const ConsoleMethod = {
  * messages on to their corresponding console counterparts (if the log method
  * is supported by the current log level)
  */
-const defaultLogHandler: LogHandler = (instance, logType, message): void => {
+const defaultLogHandler: LogHandler = (instance, logType, ...args): void => {
   if (logType < instance.logLevel) {
     return;
   }
@@ -114,7 +114,7 @@ const defaultLogHandler: LogHandler = (instance, logType, message): void => {
   if (method) {
     console[method as 'log' | 'info' | 'warn' | 'error'](
       `[${now}]  ${instance.name}:`,
-      message
+      ...args
     );
   } else {
     throw new Error(
@@ -215,11 +215,6 @@ export function setUserLogHandler(
   logCallback: LogCallback | null,
   options?: LogOptions
 ): void {
-  if (logCallback !== null && typeof logCallback !== 'function') {
-    throw new TypeError(
-      'First argument to `onLog` must be null or a function.'
-    );
-  }
   for (const instance of instances) {
     let threshhold = instance.logLevel;
     if (options && options.level) {
diff --git a/packages/logger/test/custom-logger.test.ts b/packages/logger/test/custom-logger.test.ts
index 167b581b922..ffd4257f586 100644
--- a/packages/logger/test/custom-logger.test.ts
+++ b/packages/logger/test/custom-logger.test.ts
@@ -51,6 +51,7 @@ describe(`Custom log handler`, () => {
     });
 
     it('calls custom callback with correct data for calling instance', () => {
+      // Default log level is INFO.
       client1.info('info message!');
       expect(result.message).to.equal('info message!');
       expect(result.args[0]).to.equal('info message!');
@@ -67,6 +68,7 @@ describe(`Custom log handler`, () => {
     });
 
     it('parses multiple arguments correctly', () => {
+      // Default log level is INFO.
       client1.info('info message!', ['hello'], 1, { a: 3 });
       expect(result.message).to.equal('info message! ["hello"] 1 {"a":3}');
       expect(result.args).to.deep.equal(['info message!', ['hello'], 1, { a: 3 }]);
@@ -75,7 +77,8 @@ describe(`Custom log handler`, () => {
     });
 
     it('calls custom callback when log call is above set log level', () => {
-      // Info was already tested above.
+      // Default log level is INFO.
+      // INFO was already tested above.
       client1.warn('warning message!');
       expect(result.message).to.equal('warning message!');
       expect(result.level).to.equal('warn');
@@ -87,6 +90,7 @@ describe(`Custom log handler`, () => {
     });
   
     it('does not call custom callback when log call is not above set log level', () => {
+      // Default log level is INFO.
       client1.log('message you should not see');
       expect(result).to.be.null;
       expect(spies.logSpy.called).to.be.false;

From 9d8f2a54c0dc99e80c028eea6d150fc8edc3172e Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Mon, 9 Mar 2020 14:52:05 -0700
Subject: [PATCH 12/13] Address more PR comments

---
 packages/app/src/firebaseNamespaceCore.ts | 25 +++++++++++++++--------
 packages/logger/src/logger.ts             |  6 +++---
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/packages/app/src/firebaseNamespaceCore.ts b/packages/app/src/firebaseNamespaceCore.ts
index b28b3880bb8..5d5e25989c0 100644
--- a/packages/app/src/firebaseNamespaceCore.ts
+++ b/packages/app/src/firebaseNamespaceCore.ts
@@ -34,7 +34,12 @@ import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
 import { DEFAULT_ENTRY_NAME, PLATFORM_LOG_STRING } from './constants';
 import { version } from '../../firebase/package.json';
 import { logger } from './logger';
-import { setUserLogHandler, setLogLevel, LogCallback, LogOptions } from '@firebase/logger';
+import {
+  setUserLogHandler,
+  setLogLevel,
+  LogCallback,
+  LogOptions
+} from '@firebase/logger';
 import { Component, ComponentType, Name } from '@firebase/component';
 
 /**
@@ -62,14 +67,7 @@ export function createFirebaseNamespaceCore(
     app,
     registerVersion,
     setLogLevel,
-    onLog: (
-      logCallback: LogCallback | null,
-      options?: LogOptions) => {
-        if (logCallback !== null && typeof logCallback !== 'function') {
-          throw ERROR_FACTORY.create(AppError.INVALID_LOG_ARGUMENT, { appName: name });
-        }
-        setUserLogHandler(logCallback, options);
-      },
+    onLog,
     // @ts-ignore
     apps: null,
     SDK_VERSION: version,
@@ -287,6 +285,15 @@ export function createFirebaseNamespaceCore(
     );
   }
 
+  function onLog(logCallback: LogCallback | null, options?: LogOptions): void {
+    if (logCallback !== null && typeof logCallback !== 'function') {
+      throw ERROR_FACTORY.create(AppError.INVALID_LOG_ARGUMENT, {
+        appName: name
+      });
+    }
+    setUserLogHandler(logCallback, options);
+  }
+
   // Map the requested service to a registered service name
   // (used to map auth to serverAuth service when needed).
   function useAsService(app: FirebaseApp, name: string): string | null {
diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts
index f83f390bf1e..14e7ab88ae6 100644
--- a/packages/logger/src/logger.ts
+++ b/packages/logger/src/logger.ts
@@ -216,9 +216,9 @@ export function setUserLogHandler(
   options?: LogOptions
 ): void {
   for (const instance of instances) {
-    let threshhold = instance.logLevel;
+    let customLogLevel: LogLevel | null = null;
     if (options && options.level) {
-      threshhold = levelStringToEnum[options.level];
+      customLogLevel = levelStringToEnum[options.level];
     }
     if (logCallback === null) {
       instance.userLogHandler = null;
@@ -248,7 +248,7 @@ export function setUserLogHandler(
           })
           .filter(arg => arg)
           .join(' ');
-        if (level >= threshhold) {
+        if (level >= (customLogLevel ?? instance.logLevel)) {
           logCallback({
             level: LogLevel[level].toLowerCase() as LogLevelString,
             message,

From 8cc455b67252cb03b5a1ccecc072fb9ce3f0692a Mon Sep 17 00:00:00 2001
From: Christina Holland <hsubox@gmail.com>
Date: Wed, 11 Mar 2020 09:28:27 -0700
Subject: [PATCH 13/13] Fix comment

---
 packages/firebase/index.d.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/firebase/index.d.ts b/packages/firebase/index.d.ts
index 998be90fbc3..bf6e53dff43 100644
--- a/packages/firebase/index.d.ts
+++ b/packages/firebase/index.d.ts
@@ -133,7 +133,7 @@ declare namespace firebase {
        */
       args: unknown[];
       /**
-       * A string indicating the source of the log call, usually a package name
+       * A string indicating the name of the package that made the log call,
        * such as `@firebase/firestore`.
        */
       type: string;