Skip to content

Commit

Permalink
remove debug and use id to identify dataloader action
Browse files Browse the repository at this point in the history
  • Loading branch information
kou_hin committed Dec 21, 2016
1 parent d541c15 commit 9d6ff56
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 139 deletions.
44 changes: 24 additions & 20 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -77,7 +73,15 @@
"plugins": [
"react",
"import"
]
],
"rules": {
"import/no-extraneous-dependencies": [
"error",
{
"devDependencies": true
}
]
}
},
"babel": {
"presets": [
Expand Down
32 changes: 1 addition & 31 deletions src/data-loader.js
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);

Expand All @@ -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));
Expand All @@ -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));
Expand Down Expand Up @@ -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;
}

Expand Down
12 changes: 5 additions & 7 deletions src/load.js
View file
Original file line number Diff line number Diff line change
@@ -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;
}
28 changes: 3 additions & 25 deletions src/middleware.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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);
}
Expand Down
4 changes: 0 additions & 4 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
53 changes: 26 additions & 27 deletions test/data-loader.test.js
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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 */
7 changes: 5 additions & 2 deletions test/load.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
7 changes: 1 addition & 6 deletions test/middleware.test.js
View file
Original file line number Diff line number Diff line change
@@ -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: () => {},
Expand Down
Loading

0 comments on commit 9d6ff56

Please sign in to comment.