Merge pull request #23 from kouhin/feature/simplify_action_matching

Simplify souce code
kouhin · Dec 21, 2016 · 83ef67f · 83ef67f
2 parents d541c15 + 9d6ff56
commit 83ef67f
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 139 deletions.
diff --git a/package.json b/package.json
@@ -41,30 +41,26 @@
   "author": "Bin Hou",
   "license": "MIT",
   "devDependencies": {
-    "babel-cli": "^6.10.1",
-    "babel-core": "^6.10.4",
-    "babel-eslint": "^6.1.2",
-    "babel-loader": "^6.2.4",
-    "babel-plugin-transform-runtime": "^6.9.0",
-    "babel-preset-env": "0.0.9",
-    "babel-preset-es2015": "^6.9.0",
-    "babel-preset-react": "^6.11.1",
-    "babel-preset-stage-0": "^6.5.0",
+    "babel-cli": "^6.18.0",
+    "babel-core": "^6.21.0",
+    "babel-eslint": "^7.1.1",
+    "babel-loader": "^6.2.10",
+    "babel-polyfill": "^6.20.0",
+    "babel-preset-es2015": "^6.18.0",
+    "babel-preset-react": "^6.16.0",
+    "babel-preset-stage-0": "^6.16.0",
     "chai": "^3.5.0",
-    "chai-as-promised": "^5.3.0",
     "eslint-config-airbnb-deps": "^13.0.0",
     "isparta": "^4.0.0",
-    "istanbul": "^0.4.4",
-    "mocha": "^2.5.3",
-    "rimraf": "^2.5.3",
-    "sinon": "^1.17.4",
-    "sinon-chai": "^2.8.0",
-    "webpack": "^1.13.1"
+    "istanbul": "^0.4.5",
+    "mocha": "^3.2.0",
+    "rimraf": "^2.5.4",
+    "sinon": "^1.17.6",
+    "webpack": "^1.14.0"
   },
   "dependencies": {
-    "babel-polyfill": "^6.9.1",
-    "debug": "^2.2.0",
-    "lodash": "^4.13.1"
+    "babel-runtime": "^6.20.0",
+    "lodash": "^4.17.2"
   },
   "eslintConfig": {
     "parser": "babel-eslint",
@@ -77,7 +73,15 @@
     "plugins": [
       "react",
       "import"
-    ]
+    ],
+    "rules": {
+      "import/no-extraneous-dependencies": [
+        "error",
+        {
+          "devDependencies": true
+        }
+      ]
+    }
   },
   "babel": {
     "presets": [

diff --git a/src/data-loader.js b/src/data-loader.js
@@ -1,12 +1,9 @@
 import isEqual from 'lodash/isEqual';
-import Debug from 'debug';
 
 import { loadFailure, loadSuccess } from './action';
 import { isAction } from './utils';
 import { fixedWait } from './wait-strategies';
 
-const debug = new Debug('redux-dataloader:data-loader');
-
 const DEFAULT_OPTIONS = {
   ttl: 10000, // Default TTL: 10s
   retryTimes: 1,
@@ -56,58 +53,36 @@ class DataLoaderTask {
 
     const disableInternalAction = !!options.disableInternalAction;
 
-    if (debug.enabled) {
-      debug('Excute with options', opts);
-    }
-
     if (!this.params.shouldFetch(this.context)) {
-      if (debug.enabled) {
-        debug('shouldFetch() returns false');
-      }
       if (!disableInternalAction) {
         const successAction = loadSuccess(this.context.action);
         this.context.dispatch(successAction); // load nothing
-        if (debug.enabled) {
-          debug('A success action is dispatched for shouldFetch() = false', successAction);
-        }
       }
       return null;
     }
     const loadingAction = this.params.loading(this.context);
     this.context.dispatch(loadingAction);
-    if (debug.enabled) {
-      debug('A loading action is dispatched', loadingAction);
-    }
 
     let currentRetry = 0;
     let result;
     let error;
 
     for (;;) {
       try {
-        if (debug.enabled) {
-          debug('Start fetching, try = ', (currentRetry + 1));
-        }
         result = await this.params.fetch(this.context);
-        if (debug.enabled) {
-          debug('Fetching success, result = ', result);
-        }
         break;
       } catch (ex) {
-        debug('Fetching failed, ex = ', ex);
         currentRetry += 1;
         if (options.retryTimes && currentRetry < opts.retryTimes) {
           const sleepTime = opts.retryWait.next().value;
-          if (debug.enabled) {
-            debug(`Sleeping for ${sleepTime} ms..., and retry`);
-          }
           await sleep(sleepTime);
         } else {
           error = ex;
           break;
         }
       }
     }
+
     if (!error) {
       const successAction = this.params.success(this.context, result);
 
@@ -124,7 +99,6 @@ class DataLoaderTask {
         return errorAction;
       }
 
-      debug('Dispatch a success action', successAction);
       this.context.dispatch(successAction);
       if (!disableInternalAction) {
         this.context.dispatch(loadSuccess(this.context.action, result));
@@ -142,7 +116,6 @@ class DataLoaderTask {
       }
       return errorAction;
     }
-    debug('Dispatch an error action', errorAction);
     this.context.dispatch(errorAction);
     if (!disableInternalAction) {
       this.context.dispatch(loadFailure(this.context.action, error));
@@ -192,9 +165,6 @@ class DataLoaderTaskDescriptor {
  */
 function createLoader(pattern, params, options) {
   const dataLoaderDescriptor = new DataLoaderTaskDescriptor(pattern, params, options);
-  if (debug.enabled) {
-    debug('Create a new data loader descriptor', dataLoaderDescriptor);
-  }
   return dataLoaderDescriptor;
 }
 

diff --git a/src/load.js b/src/load.js
@@ -1,16 +1,14 @@
-import Debug from 'debug';
 import { loadRequest } from './action';
 import { isAction } from './utils';
 
-const debug = new Debug('redux-dataloader:load');
+export const DATALOADER_ACTION_ID = () => {};
 
 export default function load(action) {
   if (!isAction(action)) {
     throw new Error('action must be object', action);
   }
-  if (debug.enabled) {
-    debug('load() an action = ', action);
-    debug('A Promise with a wrapped action is returned', loadRequest(action));
-  }
-  return Promise.resolve(loadRequest(action));
+  const asyncAction = Promise.resolve(loadRequest(action));
+  // eslint-disable-next-line no-underscore-dangle
+  asyncAction._id = DATALOADER_ACTION_ID;
+  return asyncAction;
 }
diff --git a/src/middleware.js b/src/middleware.js
@@ -2,12 +2,8 @@ import findKey from 'lodash/findKey';
 import find from 'lodash/find';
 import isEqual from 'lodash/isEqual';
 import isInteger from 'lodash/isInteger';
-import Debug from 'debug';
 
-import { isPromise } from './utils';
-import { LOAD_DATA_REQUEST_ACTION } from './action';
-
-const debug = new Debug('redux-dataloader:middleware');
+import { DATALOADER_ACTION_ID } from './load';
 
 function findRunningTaskKey(runningTasksMap, action) {
   return findKey(runningTasksMap, o => isEqual(o.action, action));
@@ -28,29 +24,20 @@ export default function createDataLoaderMiddleware(loaders, args, opts) {
     };
 
     return next => (receivedAction) => {
-      if (!isPromise(receivedAction)) {
+      // eslint-disable-next-line no-underscore-dangle
+      if (!receivedAction._id || receivedAction._id !== DATALOADER_ACTION_ID) {
         return next(receivedAction);
       }
-      debug('Received a promise action', receivedAction);
 
       return receivedAction.then((asyncAction) => {
-        if (asyncAction.type !== LOAD_DATA_REQUEST_ACTION) {
-          debug(`Received promise action is not ${LOAD_DATA_REQUEST_ACTION}, pass it to the next middleware`); // eslint-disable-line max-len
-          return next(receivedAction);
-        }
-        debug(`Received promise action is ${LOAD_DATA_REQUEST_ACTION}, pass wrapped action to the next middleware`); // eslint-disable-line max-len
         next(asyncAction); // dispatch data loader request action
         const { action } = asyncAction.meta;
-        debug('Original action is', action);
         const runningTaskKey = findRunningTaskKey(runningTasks, action);
-        debug('Find task from task cache');
         if (runningTaskKey) {
-          debug('Cache hit!, Key = ', runningTaskKey);
           return runningTasks[runningTaskKey].promise;
         }
 
         const taskDescriptor = find(loaders, loader => loader.supports(action));
-        debug('Cache does not hit, finding task descriptor', taskDescriptor);
 
         if (!taskDescriptor) {
           throw new Error('No loader for action', action);
@@ -61,15 +48,8 @@ export default function createDataLoaderMiddleware(loaders, args, opts) {
           ...taskDescriptor.options,
           ...(asyncAction.meta.options || {}),
         };
-        debug(
-          'Merge options from taskDescriptor and dispatched action',
-          taskDescriptor.options,
-          asyncAction.meta.options, options,
-        );
 
         const key = uniqueId(`${action.type}__`);
-        debug('Generate new cache key', key);
-        debug('Start executing');
         const runningTask = taskDescriptor.newTask(ctx, action).execute(options);
 
         if (isInteger(options.ttl) && options.ttl > 0) {
@@ -78,9 +58,7 @@ export default function createDataLoaderMiddleware(loaders, args, opts) {
             promise: runningTask,
           };
           if (typeof window !== 'undefined' && typeof document !== 'undefined') {
-            debug(`Set cache ttl for task[${key}], ttl = ${options.ttl}`);
             setTimeout(() => {
-              debug(`Task[${key}] is removed from cache, for ttl = ${options.ttl} ms`);
               delete runningTasks[key];
             }, options.ttl);
           }

diff --git a/src/utils.js b/src/utils.js
@@ -1,7 +1,3 @@
-export function isPromise(val) {
-  return val && typeof val.then === 'function';
-}
-
 export function isAction(action) {
   const result = action && (typeof action) === 'object' &&
         action.type && (typeof action.type) === 'string';

diff --git a/test/data-loader.test.js b/test/data-loader.test.js
@@ -1,15 +1,10 @@
 /* eslint-disable no-unused-expressions */
-import chai, { expect } from 'chai';
-import sinonChai from 'sinon-chai';
-import chaiAsPromised from 'chai-as-promised';
+import { expect } from 'chai';
 import { describe, it } from 'mocha';
 import sinon from 'sinon';
 
 import { createLoader } from '../src';
 
-chai.use(sinonChai);
-chai.use(chaiAsPromised);
-
 describe('test createLoader: action matcher', () => {
   const loader = {
     success: () => null,
@@ -117,14 +112,16 @@ describe('test createLoader: DataLoderTask', () => {
       },
     }).execute();
 
-    return expect(promise.then(() => {
-      expect(loadingSpy).to.have.been.calledOnce;
-      expect(shouldFetchSpy).to.have.been.calledOnce;
-      expect(fetchSpy).to.have.been.calledOnce;
-      expect(successSpy).to.have.been.calledOnce;
-      expect(errorSpy).to.have.not.been.called;
+    promise.then(() => {
+      expect(loadingSpy.calledOnce).to.be.true;
+      expect(shouldFetchSpy.calledOnce).to.be.true;
+      expect(shouldFetchSpy.calledOnce).to.be.true;
+      expect(fetchSpy.calledOnce).to.be.true;
+      expect(successSpy.calledOnce).to.be.true;
+      expect(errorSpy.notCalled).to.be.true;
       sinon.assert.callOrder(shouldFetchSpy, fetchSpy, successSpy);
-    })).to.be.fulfilled.notify(done);
+      done();
+    }, done);
   });
 
   it('loading -> shouldFetch(return false) -> noop', (done) => {
@@ -154,13 +151,14 @@ describe('test createLoader: DataLoderTask', () => {
       },
     }).execute();
 
-    return expect(promise.then(() => {
-      expect(shouldFetchSpy).to.have.been.calledOnce;
-      expect(loadingSpy).to.have.not.been.called;
-      expect(fetchSpy).to.have.not.been.calledOnce;
-      expect(successSpy).to.have.not.been.called;
-      expect(errorSpy).to.have.not.been.called;
-    })).to.be.fulfilled.notify(done);
+    promise.then(() => {
+      expect(shouldFetchSpy.calledOnce).to.be.true;
+      expect(loadingSpy.notCalled).to.be.true;
+      expect(fetchSpy.notCalled).to.be.true;
+      expect(successSpy.notCalled).to.be.true;
+      expect(errorSpy.notCalled).to.be.true;
+      done();
+    }, done);
   });
 
   it('loading -> shouldFetch -> fetch -> error', (done) => {
@@ -190,14 +188,15 @@ describe('test createLoader: DataLoderTask', () => {
       },
     }).execute();
 
-    return expect(promise.then(() => {
-      expect(loadingSpy).to.have.been.calledOnce;
-      expect(shouldFetchSpy).to.have.been.calledOnce;
-      expect(fetchSpy).to.have.been.calledOnce;
-      expect(successSpy).to.have.not.been.called;
-      expect(errorSpy).to.have.been.calledOnce;
+    promise.then(() => {
+      expect(loadingSpy.calledOnce).to.be.true;
+      expect(shouldFetchSpy.calledOnce).to.be.true;
+      expect(fetchSpy.calledOnce).to.be.true;
+      expect(successSpy.notCalled).to.be.true;
+      expect(errorSpy.calledOnce).to.be.true;
       sinon.assert.callOrder(shouldFetchSpy, fetchSpy, errorSpy);
-    })).to.be.fulfilled.notify(done);
+      done();
+    }, done);
   });
 });
 /* eslint-enable no-unused-expressions */
diff --git a/test/load.test.js b/test/load.test.js
@@ -11,15 +11,18 @@ describe('load()', () => {
     },
   };
 
-  it('wrap an action, should return promise', () => {
+  it('wrap an action, should return promise', (done) => {
     const promise = load(requestAction);
     const expected = {
       type: LOAD_DATA_REQUEST_ACTION,
       meta: {
         action: requestAction,
       },
     };
-    return expect(promise).to.eventually.deep.equal(expected);
+    promise.then((result) => {
+      expect(result).to.be.deep.equal(expected);
+      done();
+    }, done);
   });
 
   it('pass a non-object to load(), should throw an Error', () => {

diff --git a/test/middleware.test.js b/test/middleware.test.js
@@ -1,13 +1,8 @@
-import chai, { expect } from 'chai';
-import sinonChai from 'sinon-chai';
-import chaiAsPromised from 'chai-as-promised';
+import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import { createLoader, createDataLoaderMiddleware } from '../src';
 
-chai.use(sinonChai);
-chai.use(chaiAsPromised);
-
 describe('createDataLoaderMiddleware()', () => {
   const loaderObj = {
     success: () => {},