From 08f504bd728b6104611cf3da7d184074d5a2853b Mon Sep 17 00:00:00 2001 From: Meagan Fenisey-Copes Date: Sat, 15 Apr 2017 10:45:28 -0400 Subject: [PATCH 1/5] Refactor index.js --- src/index.js | 105 ++++++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/src/index.js b/src/index.js index 158204f..4fce642 100644 --- a/src/index.js +++ b/src/index.js @@ -21,27 +21,29 @@ import defaults from './defaults'; * * @returns {function} logger middleware */ -function createLogger(options = {}) { - const loggerOptions = Object.assign({}, defaults, options); +function directlyApplied(options) { + return options.getState && options.dispatch; +} - const { - logger, - stateTransformer, - errorTransformer, - predicate, - logErrors, - diffPredicate, - } = loggerOptions; +function noLogger(options) { + return typeof options.logger === 'undefined'; +} - // Return if 'console' object is not defined - if (typeof logger === 'undefined') { - return () => next => action => next(action); - } +function shouldNotLog({ predicate }, getState, action) { + return typeof predicate === 'function' && !predicate(getState, action); +} - // Detect if 'createLogger' was passed directly to 'applyMiddleware'. - if (options.getState && options.dispatch) { - // eslint-disable-next-line no-console - console.error(`[redux-logger] redux-logger not installed. Make sure to pass logger instance as middleware: +function shouldDiff({ diff, diffPredicate }, getState, action) { + return diff && typeof diffPredicate === 'function' && diffPredicate(getState, action); +} + +function emptyLogger() { + return () => next => action => next(action); +} + +function emptyLoggerWarning() { + // eslint-disable-next-line no-console + console.error(`[redux-logger] redux-logger not installed. Make sure to pass logger instance as middleware: // Logger with default options import { logger } from 'redux-logger' const store = createStore( @@ -58,49 +60,58 @@ const store = createStore( applyMiddleware(logger) ) `); +} - return () => next => action => next(action); +function createLogger(options = {}) { + // Detect if 'createLogger' was passed directly to 'applyMiddleware'. + if (directlyApplied(options)) { + emptyLoggerWarning(); + return emptyLogger(); } - const logBuffer = []; + // Return if 'console' object is not defined + if (noLogger(options)) return emptyLogger(); + + const loggerOptions = Object.assign({}, defaults, options): return ({ getState }) => next => (action) => { // Exit early if predicate function returns 'false' - if (typeof predicate === 'function' && !predicate(getState, action)) { - return next(action); - } - - const logEntry = {}; - - logBuffer.push(logEntry); - - logEntry.started = timer.now(); - logEntry.startedTime = new Date(); - logEntry.prevState = stateTransformer(getState()); - logEntry.action = action; + if (shouldNotLog(getState, action)) return next(action); + const started = timer.now(); + const startedTime = new Date(); + const prevState = loggerOptions.stateTransformer(getState()); let returnedValue; - if (logErrors) { - try { - returnedValue = next(action); - } catch (e) { - logEntry.error = errorTransformer(e); - } - } else { + let error; + + try { returnedValue = next(action); + } catch (e) { + if (loggerOptions.logErrors) { + error = loggerOptions.errorTransformer(e); + } else { + throw e; + } } - logEntry.took = timer.now() - logEntry.started; - logEntry.nextState = stateTransformer(getState()); + const took = timer.now() - started; + const nextState = loggerOptions.stateTransformer(getState()); - const diff = loggerOptions.diff && typeof diffPredicate === 'function' - ? diffPredicate(getState, action) - : loggerOptions.diff; + loggerOptions.diff = shouldDiff(loggerOptions, getState, action); - printBuffer(logBuffer, Object.assign({}, loggerOptions, { diff })); - logBuffer.length = 0; + printBuffer( + [{ + started, + startedTime, + prevState, + action, + error, + took, + nextState, + }], + loggerOptions); - if (logEntry.error) throw logEntry.error; + if (error) throw error; return returnedValue; }; } From 0e9cbeeb13fc510e149ab61c8a31d952ae819caf Mon Sep 17 00:00:00 2001 From: Meagan Fenisey-Copes Date: Sat, 15 Apr 2017 12:10:31 -0400 Subject: [PATCH 2/5] Fix noLogger ordering --- src/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index 4fce642..edb6403 100644 --- a/src/index.js +++ b/src/index.js @@ -69,11 +69,11 @@ function createLogger(options = {}) { return emptyLogger(); } - // Return if 'console' object is not defined - if (noLogger(options)) return emptyLogger(); - const loggerOptions = Object.assign({}, defaults, options): + // Return if 'console' object is not defined + if (noLogger(loggerOptions)) return emptyLogger(); + return ({ getState }) => next => (action) => { // Exit early if predicate function returns 'false' if (shouldNotLog(getState, action)) return next(action); From a32491f6f9171586dd65ffaae35b0e490b9be5ed Mon Sep 17 00:00:00 2001 From: Meagan Fenisey-Copes Date: Sat, 15 Apr 2017 12:19:53 -0400 Subject: [PATCH 3/5] Have option checkers return boolean --- src/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index edb6403..89eb66b 100644 --- a/src/index.js +++ b/src/index.js @@ -22,19 +22,19 @@ import defaults from './defaults'; * @returns {function} logger middleware */ function directlyApplied(options) { - return options.getState && options.dispatch; + return !!(options.getState && options.dispatch); } function noLogger(options) { - return typeof options.logger === 'undefined'; + return !options.logger; } function shouldNotLog({ predicate }, getState, action) { - return typeof predicate === 'function' && !predicate(getState, action); + return !!(typeof predicate === 'function' && !predicate(getState, action)); } function shouldDiff({ diff, diffPredicate }, getState, action) { - return diff && typeof diffPredicate === 'function' && diffPredicate(getState, action); + return !!(diff && typeof diffPredicate === 'function' && diffPredicate(getState, action)); } function emptyLogger() { From e6c039a4843247757b9d983e02c4347a71cc7eb2 Mon Sep 17 00:00:00 2001 From: Meagan Fenisey-Copes Date: Sun, 16 Apr 2017 10:04:05 -0400 Subject: [PATCH 4/5] Add more specs/reorganize --- spec/helpers.spec.js | 2 +- spec/index.spec.js | 47 ++++++++++++++++++++++++++++++++++---------- src/index.js | 4 ++-- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/spec/helpers.spec.js b/spec/helpers.spec.js index 7d8d4c2..408a628 100644 --- a/spec/helpers.spec.js +++ b/spec/helpers.spec.js @@ -18,6 +18,6 @@ context('Helpers', () => { it('should format a time given a Date object', () => { const time = new Date('December 25, 1995 23:15:30'); expect(formatTime(time)).to.equal('23:15:30.000'); - }); + }); }); }); diff --git a/spec/index.spec.js b/spec/index.spec.js index 0373ef6..3a48d65 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -22,27 +22,54 @@ context('default logger', () => { }); context('createLogger', () => { - describe('init', () => { + beforeEach(() => { + sinon.spy(console, 'error'); + sinon.spy(console, 'log'); + }); + + afterEach(() => { + console.error.restore(); + console.log.restore(); + }); + + let store; + + context('mistakenly passed directly to applyMiddleware', () => { beforeEach(() => { - sinon.spy(console, 'error'); + store = createStore(() => ({}), applyMiddleware(createLogger)); }); - afterEach(() => { - console.error.restore(); + it('should log error', () => { + sinon.assert.calledOnce(console.error); }); - it('should throw error if passed direct to applyMiddleware', () => { - const store = createStore(() => ({}), applyMiddleware(createLogger)); + it('should create an empty middleware', () => { + store.dispatch({ type: 'foo' }); + sinon.assert.notCalled(console.log); + }); + }); + + context('options.logger undefined or null', () => { + beforeEach(() => { + const logger = createLogger({ logger: null }); + store = createStore(() => ({}), applyMiddleware(logger)); + }); + it('should create an empty middleware', () => { store.dispatch({ type: 'foo' }); - sinon.assert.calledOnce(console.error); + sinon.assert.notCalled(console.log); }); + }); - it('should be ok', () => { - const store = createStore(() => ({}), applyMiddleware(createLogger())); + context('options.predicate returns false', () => { + beforeEach(() => { + const logger = createLogger({ predicate: () => false }); + store = createStore(() => ({}), applyMiddleware(logger)); + }); + it('should not log', () => { store.dispatch({ type: 'foo' }); - sinon.assert.notCalled(console.error); + sinon.assert.notCalled(console.log); }); }); }); diff --git a/src/index.js b/src/index.js index 89eb66b..9d47c09 100644 --- a/src/index.js +++ b/src/index.js @@ -69,14 +69,14 @@ function createLogger(options = {}) { return emptyLogger(); } - const loggerOptions = Object.assign({}, defaults, options): + const loggerOptions = Object.assign({}, defaults, options); // Return if 'console' object is not defined if (noLogger(loggerOptions)) return emptyLogger(); return ({ getState }) => next => (action) => { // Exit early if predicate function returns 'false' - if (shouldNotLog(getState, action)) return next(action); + if (shouldNotLog(options, getState, action)) return next(action); const started = timer.now(); const startedTime = new Date(); From ace19b867346a10967cd84ddea25174989cf8222 Mon Sep 17 00:00:00 2001 From: Meagan Fenisey-Copes Date: Fri, 19 May 2017 08:26:14 -0400 Subject: [PATCH 5/5] noLogger -> hasLogger --- src/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index 9d47c09..cb86992 100644 --- a/src/index.js +++ b/src/index.js @@ -25,8 +25,8 @@ function directlyApplied(options) { return !!(options.getState && options.dispatch); } -function noLogger(options) { - return !options.logger; +function hasLogger(options) { + return options.logger; } function shouldNotLog({ predicate }, getState, action) { @@ -72,7 +72,7 @@ function createLogger(options = {}) { const loggerOptions = Object.assign({}, defaults, options); // Return if 'console' object is not defined - if (noLogger(loggerOptions)) return emptyLogger(); + if (!hasLogger(loggerOptions)) return emptyLogger(); return ({ getState }) => next => (action) => { // Exit early if predicate function returns 'false'