core: Add an option to supply logger with debug, warn and error methods (#1661)

* Add an option to supply customLogger that will receive message and type from uppy.log

* Always use logger function, set to nullLogger by default

Co-Authored-By: Renée Kooi <github@kooi.me>

* add tests for logging and not logging with a function and debug: true

* Update packages/@uppy/core/src/loggers.js

account for `debug` not being available in IE10

Co-Authored-By: Renée Kooi <renee@kooi.me>

* add .call

* fix tests

* expose Uppy.debugLogger

* document logger

* core cleanup

* add logger to defaultOptions in docs too

* debug: true — only override logger if it has not been set in opts

packages/@uppy/core/src/index.js

@@ -8,8 +8,8 @@ const DefaultStore = require('@uppy/store-default')
 const getFileType = require('@uppy/utils/lib/getFileType')
 const getFileNameAndExtension = require('@uppy/utils/lib/getFileNameAndExtension')
 const generateFileID = require('@uppy/utils/lib/generateFileID')
-const getTimeStamp = require('@uppy/utils/lib/getTimeStamp')
 const supportsUploadProgress = require('./supportsUploadProgress')
+const { nullLogger, debugLogger } = require('./loggers')
 const Plugin = require('./Plugin') // Exported from here.
 
 class RestrictionError extends Error {
@@ -89,13 +89,22 @@ class Uppy {
       meta: {},
       onBeforeFileAdded: (currentFile, files) => currentFile,
       onBeforeUpload: (files) => files,
-      store: DefaultStore()
+      store: DefaultStore(),
+      logger: nullLogger
     }
 
     // Merge default options with the ones set by user
     this.opts = Object.assign({}, defaultOptions, opts)
     this.opts.restrictions = Object.assign({}, defaultOptions.restrictions, this.opts.restrictions)
 
+    // Support debug: true for backwards-compatability, unless logger is set in opts
+    // opts instead of this.opts to avoid comparing objects — we set logger: nullLogger in defaultOptions
+    if (opts && opts.logger && opts.debug) {
+      this.log('You are using a custom `logger`, but also set `debug: true`, which uses built-in logger to output logs to console. Ignoring `debug: true` and using your custom `logger`.', 'warning')
+    } else if (opts && opts.debug) {
+      this.opts.logger = debugLogger
+    }
+
     this.log(`Using Core v${this.constructor.VERSION}`)
 
     // i18n
@@ -170,10 +179,8 @@ class Uppy {
       this.updateAll(nextState)
     })
 
-    // for debugging and testing
-    // this.updateNum = 0
+    // Exposing uppy object on window for debugging and testing
     if (this.opts.debug && typeof window !== 'undefined') {
-      window['uppyLog'] = ''
       window[this.opts.id] = this
     }
 
@@ -320,7 +327,7 @@ class Uppy {
   setFileMeta (fileID, data) {
     const updatedFiles = Object.assign({}, this.getState().files)
     if (!updatedFiles[fileID]) {
-      this.log('Was trying to set metadata for a file that’s not with us anymore: ', fileID)
+      this.log('Was trying to set metadata for a file that has been removed: ', fileID)
       return
     }
     const newMeta = Object.assign({}, updatedFiles[fileID].meta, data)
@@ -1048,29 +1055,19 @@ class Uppy {
   }
 
   /**
-   * Logs stuff to console, only if `debug` is set to true. Silent in production.
+   * Passes messages to a function, provided in `opt.logger`.
+   * If `opt.logger: Uppy.debugLogger` or `opt.debug: true`, logs to the browser console.
    *
    * @param {string|Object} message to log
    * @param {string} [type] optional `error` or `warning`
    */
   log (message, type) {
-    if (!this.opts.debug) {
-      return
+    const { logger } = this.opts
+    switch (type) {
+      case 'error': logger.error(message); break
+      case 'warning': logger.warn(message); break
+      default: logger.debug(message); break
     }
-
-    const prefix = `[Uppy] [${getTimeStamp()}]`
-
-    if (type === 'error') {
-      console.error(prefix, message)
-      return
-    }
-
-    if (type === 'warning') {
-      console.warn(prefix, message)
-      return
-    }
-
-    console.log(prefix, message)
   }
 
   /**
@@ -1323,3 +1320,4 @@ module.exports = function (opts) {
 // Expose class constructor.
 module.exports.Uppy = Uppy
 module.exports.Plugin = Plugin
+module.exports.debugLogger = debugLogger

packages/@uppy/core/src/index.test.js

@@ -1547,4 +1547,103 @@ describe('src/Core', () => {
       expect(core.opts.restrictions.minNumberOfFiles).toBe(null)
     })
   })
+
+  describe('log', () => {
+    it('should log via provided logger function', () => {
+      const myTestLogger = {
+        debug: jest.fn(),
+        warn: jest.fn(),
+        error: jest.fn()
+      }
+
+      const core = new Core({
+        logger: myTestLogger
+      })
+
+      core.log('test test')
+      core.log('test test', 'error')
+      core.log('test test', 'error')
+      core.log('test test', 'warning')
+
+      // logger.debug should have been called 1 time above,
+      // but we call log in Core’s constructor to output VERSION, hence +1 here
+      expect(core.opts.logger.debug.mock.calls.length).toBe(2)
+      expect(core.opts.logger.error.mock.calls.length).toBe(2)
+      expect(core.opts.logger.warn.mock.calls.length).toBe(1)
+    })
+
+    it('should log via provided logger function, even if debug: true', () => {
+      const myTestLogger = {
+        debug: jest.fn(),
+        warn: jest.fn(),
+        error: jest.fn()
+      }
+
+      const core = new Core({
+        logger: myTestLogger,
+        debug: true
+      })
+
+      core.log('test test')
+      core.log('test test', 'error')
+      core.log('test test', 'error')
+      core.log('test test', 'warning')
+
+      // logger.debug should have been called 1 time above,
+      // but we call log in Core’s constructor to output VERSION, hence +1 here
+      expect(core.opts.logger.debug.mock.calls.length).toBe(2)
+      expect(core.opts.logger.error.mock.calls.length).toBe(2)
+      // logger.warn should have been called 1 time above,
+      // but we warn in Core when using both logger and debug: true, hence +1 here
+      expect(core.opts.logger.warn.mock.calls.length).toBe(2)
+    })
+
+    it('should log to console when logger: Uppy.debugLogger or debug: true is set', () => {
+      console.debug = jest.fn()
+      console.error = jest.fn()
+
+      const core = new Core({
+        logger: Core.debugLogger
+      })
+
+      core.log('test test')
+      core.log('beep boop')
+      core.log('beep beep', 'error')
+
+      // console.debug debug should have been called 2 times above,
+      // ibut we call log n Core’ constructor to output VERSION, hence +1 here
+      expect(console.debug.mock.calls.length).toBe(3)
+      expect(console.error.mock.calls.length).toBe(1)
+
+      console.debug.mockClear()
+      console.error.mockClear()
+
+      const core2 = new Core({
+        debug: true
+      })
+
+      core2.log('test test')
+      core2.log('beep boop')
+      core2.log('beep beep', 'error')
+
+      // console.debug debug should have been called 2 times here,
+      // but we call log in Core constructor to output VERSION, hence +1 here
+      expect(console.debug.mock.calls.length).toBe(3)
+      expect(console.error.mock.calls.length).toBe(1)
+    })
+
+    it('should not log to console when logger is not set', () => {
+      console.debug = jest.fn()
+      console.error = jest.fn()
+
+      const core = new Core()
+
+      core.log('test test')
+      core.log('beep boop')
+      core.log('beep beep', 'error')
+
+      expect(console.debug.mock.calls.length).toBe(0)
+      expect(console.error.mock.calls.length).toBe(0)
+    })
+  })
 })

packages/@uppy/core/src/loggers.js

@@ -0,0 +1,25 @@
+const getTimeStamp = require('@uppy/utils/lib/getTimeStamp')
+
+// Swallow logs, default if logger is not set or debug: false
+const nullLogger = {
+  debug: (...args) => {},
+  warn: (...args) => {},
+  error: (...args) => {}
+}
+
+// Print logs to console with namespace + timestamp,
+// set by logger: Uppy.debugLogger or debug: true
+const debugLogger = {
+  debug: (...args) => {
+    // IE 10 doesn’t support console.debug
+    const debug = console.debug || console.log
+    debug.call(console, `[Uppy] [${getTimeStamp()}]`, ...args)
+  },
+  warn: (...args) => console.warn(`[Uppy] [${getTimeStamp()}]`, ...args),
+  error: (...args) => console.error(`[Uppy] [${getTimeStamp()}]`, ...args)
+}
+
+module.exports = {
+  nullLogger,
+  debugLogger
+}

packages/uppy/index.js

@@ -1,5 +1,6 @@
 // Core
 exports.Core = require('@uppy/core')
+exports.debugLogger = exports.Core.debugLogger
 
 // Utilities
 exports.server = require('@uppy/companion-client')

website/src/docs/uppy.md

@@ -50,7 +50,8 @@ const uppy = Uppy({
   onBeforeFileAdded: (currentFile, files) => currentFile,
   onBeforeUpload: (files) => {},
   locale: {},
-  store: new DefaultStore()
+  store: new DefaultStore(),
+  logger: nullLogger
 })
 ```
 
@@ -80,6 +81,47 @@ With this option set to `true`, users can upload some files, and then add _more_
 
 With this option set to `false`, users can upload some files, and you can listen for the ['complete'](/docs/uppy/#complete) event to continue to the next step in your app's upload flow. A typical use case for this is uploading a new profile picture. If you are integrating with an existing HTML form, this option gives the closest behaviour to a bare `<input type="file">`.
 
+### `logger`
+
+An object of methods that are called with debug information from [`uppy.log`](/docs/uppy/#uppy-log).
+
+Set `logger: Uppy.debugLogger` to get debug info output to the browser console:
+
+```js
+const Uppy = require('@uppy/core')
+const uppy = Uppy({
+  logger: Uppy.debugLogger
+})
+```
+
+You can also provide your own logger object: it should expose `debug`, `warn` and `error` methods, as shown in the examples below.
+
+By default `logger` is set to `nullLogger`, which does nothing:
+
+```js
+const nullLogger = {
+  debug: (...args) => {},
+  warn: (...args) => {},
+  error: (...args) => {}
+}
+```
+
+`logger: Uppy.debugLogger` looks like this:
+
+```js
+const debugLogger = {
+  debug: (...args) => {
+    // IE 10 doesn’t support console.debug
+    const debug = console.debug || console.log
+    debug.call(console, `[Uppy] [${getTimeStamp()}]`, ...args)
+  },
+  warn: (...args) => console.warn(`[Uppy] [${getTimeStamp()}]`, ...args),
+  error: (...args) => console.error(`[Uppy] [${getTimeStamp()}]`, ...args)
+}
+```
+
+By providing your own `logger`, you can send the debug information to a server, choose to log errors only, etc.
+
 ### `restrictions: {}`
 
 Optionally, provide rules and conditions to limit the type and/or number of files that can be selected.
@@ -479,7 +521,9 @@ Uninstall all plugins and close down this Uppy instance. Also runs `uppy.reset()
 - **message** *{string}*
 - **type** *{string=}* `error` or `warning`
 
-Logs stuff to console, only if `uppy.opts.debug` is set to true. Silent in production.
+Logs stuff to [`logger`](/docs/uppy/#logger) methods.
+
+See [`logger`](/docs/uppy/#logger) docs for details.
 
 ```js
 uppy.log('[Dashboard] adding files...')