Skip to content

Commit

Permalink
Add warnings for antipatterns
Browse files Browse the repository at this point in the history
  • Loading branch information
calebmer committed Dec 17, 2015
1 parent 97cea7d commit da3b284
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 0 deletions.
27 changes: 27 additions & 0 deletions src/createEffectCapableStore.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isDevelopmentEnvironment, isPlainObject } from './utils';
import Task from './Task';
import createEnhanceReducer from './createEnhanceReducer';

Expand All @@ -11,6 +12,9 @@ import createEnhanceReducer from './createEnhanceReducer';
export default createStore => (rootReducer, initialState) => {
// Reference to the store dispatch function.
let dispatch;
let cleaningEffects = false;
let warnedPlainAction = false;
let warnedEffectEffects = false;

const cleanTask = new Task();
const effectQueue = [];
Expand All @@ -24,8 +28,10 @@ export default createStore => (rootReducer, initialState) => {
*/

const cleanEffectQueue = () => {
cleaningEffects = true;
const values = effectQueue.map(dispatch);
effectQueue.length = 0; // @see https://davidwalsh.name/empty-array
cleaningEffects = false;
return values;
};

Expand All @@ -39,6 +45,27 @@ export default createStore => (rootReducer, initialState) => {
*/

const deferEffect = effect => {
if (isDevelopmentEnvironment()) {
if (!warnedPlainAction && isPlainObject(effect)) {
warnedPlainAction = true;
console.warn(
'redux-side-effects: ' +
'Yeilding a plain object generally means you are cascading actions, ' +
'this is a banned practice (http://stackoverflow.com/questions/29309214). ' +
'Try to use thunks instead.'
);
}

if (!warnedEffectEffects && cleaningEffects) {
warnedEffectEffects = true;
console.warn(
'redux-side-effects: ' +
'It is generally a bad practice for side effects to have side effects. ' +
'Instead try to dispatch all side effects at once.'
);
}
}

effectQueue.push(effect);

if (!cleanTask.isSet()) {
Expand Down
18 changes: 18 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
/**
* Returns true if we are in a development environment.
*
* @returns {Boolean} True if in a development environment
*/

export const isDevelopmentEnvironment = () => process.env.NODE_ENV === 'development';

/**
* Simple invariant check.
*
Expand Down Expand Up @@ -30,6 +38,16 @@ export const isFunction = any => typeof any === 'function';
*/
export const isUndefined = any => typeof any === 'undefined';

/**
* Checks whether provided argument is a plain ol' object.
*
* @param {any} Anything
*
* @returns {Boolean} Result of plain object check
*/

export const isPlainObject = any => typeof any === 'object' && Object.getPrototypeOf(any) === Object.prototype;

/**
* Checks whether provided argument is iterable. The value must be defined and
* must contain function named next.
Expand Down
9 changes: 9 additions & 0 deletions test/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ describe('Utils', () => {
assert.isFalse(Utils.isIterable(false));
});

it('should use isPlainObject to detect a plain object', () => {
assert.isTrue(Utils.isPlainObject({}));
assert.isTrue(Utils.isPlainObject({ key: 'value' }));
assert.isFalse(Utils.isPlainObject(true));
assert.isFalse(Utils.isPlainObject(5));
assert.isFalse(Utils.isPlainObject('object'));
assert.isFalse(Utils.isPlainObject(new Error('Whoops!')));
});

it('will defer a callback to be called later', done => Utils.defer(done));

it('will stop a deferred callback from being called', done => {
Expand Down

0 comments on commit da3b284

Please sign in to comment.