Skip to content

Commit

Permalink
reducing API surface area
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomkis
tomkis committed Mar 23, 2016
1 parent 3b0b546 commit 875ddc6
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 193 deletions.
30 changes: 0 additions & 30 deletions src/AppStateWithEffects.js
View file

This file was deleted.

64 changes: 7 additions & 57 deletions src/createEffectCapableStore.js
View file
Original file line number Diff line number Diff line change
@@ -1,49 +1,4 @@
import { invariant, isFunction } from './utils';
import enhanceReducer from './enhanceReducer';
import AppStateWithEffects from './AppStateWithEffects';

/**
* Wraps Store `getState` method. Instead of returning just plain old state,
* it's assumed that the returned state is instance of `AppStateWithEffects`,
* therefore it proxies the call to `getAppState`.
*
* @param {Object} Original Store
* @returns {Function} Wrapped `getState`
*/
export const wrapGetState = store => () => {
const state = store.getState();

if (state instanceof AppStateWithEffects) {
return state.getAppState();
} else {
return state;
}
};

/**
* Wraps Store `dispatch` method. The original `dispatch` is called first, then
* it iterates over all the effects returned from the `getState` function.
* Every effect is then executed and dispatch is passed as the first argument.
*
* @param {Object} Original Store
* @returns {Function} Wrapped `dispatch`
*/
export const wrapDispatch = store => (...args) => {
// Let's just dispatch original action,
// the original dispatch might actually be
// enhanced by some middlewares.
const result = store.dispatch(...args);

const effects = store.getState().getEffects();

invariant(effects.every(isFunction),
`It's allowed to yield only functions (side effect)`);

// Effects are executed after action is dispatched
effects.forEach(effect => effect(wrapDispatch(store)));

return result;
};

/**
* Wraps Store `replaceReducer` method. The implementation just calls
Expand All @@ -53,28 +8,23 @@ export const wrapDispatch = store => (...args) => {
* @param {Object} Original Store
* @returns {Function} Wrapped `replaceReducer`
*/
export const wrapReplaceReducer = store => nextReducer =>
store.replaceReducer(enhanceReducer(nextReducer));
export const wrapReplaceReducer = (store, deferEffects) => nextReducer =>
store.replaceReducer(enhanceReducer(nextReducer, deferEffects));

/**
* Creates enhanced store factory, which takes original `createStore` as argument.
* The store's `dispatch` and `getState` methods are wrapped with custom implementation.
*
* wrappedDispatch calls the original dispatch and executes all the side effects.
*
* wrappedGetState unwraps original applicaiton state from `AppStateWithEffects`
* Returns modified store which is capable of handling Reducers in form of Generators.
*
* @param {Function} Original createStore implementation to be enhanced
* @returns {Function} Store factory
*/
export default createStore => (rootReducer, initialState) => {
const store = createStore(enhanceReducer(rootReducer), new AppStateWithEffects(initialState, []));
let store = null;
const deferEffects = effects => setTimeout(() => effects.forEach(effect => effect(store.dispatch)), 0);

This comment has been minimized.

Sign in to view

Copy link
@calebmer

calebmer Mar 24, 2016

To expose this via a dispatchEvents function, here's some pseudo code of what I'd try:

let effects = [];

const dispatchEffects = () => {
  const results = effects.map(effect => effect(store.dispatch)); // Dispatch effects and collect the values.
  effects = []; // Reset the queue. Note: if `deferEffects` is triggered by an effect, the resultant effects will be lost.
  return results; // Return the effect results to the user.
};

const deferEffects = nextEffects => {
  effects = effects.concat(nextEffects); // Add effects to the queue.
  setTimeout(dispatchEffects, 0); // Dispatch effects automatically asynchronously, maybe there should be a configuration option disabling automatic dispatch of effects? Or a handler to hook into this life cycle event.
};

Looking at my old code this is in essence what I had to implement dispatchEffects. The rest involved warnings. Here is another approach which (I think) might be lest performant, but it doesn't keep resetting the effects array reference and it supports effects of effects (anti-pattern or not).

const effects = [];

const dispatchEffects = () => {
  const results = []; // Create a results array.
  while (let effect = effects.shift()) { // Drain all of the effects. If an effect is added to the array while this loop is running, it should also be executed as this loop will not stop until it is empty.
    results.push(effect(store.dispatch)); // Add the result of dispatching the effect to the results.
  }
  return results;
};

const deferEffects = nextEffects => {
  nextEffects.forEach(effect => effects.push(effect)); // Add effects to the queue.
  setTimeout(dispatchEffects, 0); // Dispatch effects automatically asynchronously, maybe there should be a configuration option disabling automatic dispatch of effects? Or a handler to hook into this life cycle event.
};

I didn't test this so you'll have to check the syntax of my while loop. I've been writing a lot of rust recently, so I might have gotten my syntaxes messed up.

A couple other notes, in my original code the reason I added a Task class is to cancel the timeout if dispatchEffects was called. I no longer think that is necessary, but I feel it's important to note here. I had also added some warnings (per your request if I remember correctly), these warnings can still be helpful in this code if you feel it is worth adding the extra logic.

This comment has been minimized.

Sign in to view

Copy link
@tomkis

tomkis Mar 24, 2016

Author Contributor

Thanks for the idea!

Regarding warnings:

Both of them are probably obsolete today because in later commits I've introduced concept of sideEffect which will help with testability.
store = createStore(enhanceReducer(rootReducer, deferEffects), initialState);

return {
...store,
dispatch: wrapDispatch(store),
getState: wrapGetState(store),
liftGetState: () => store.getState(),
replaceReducer: wrapReplaceReducer(store)
replaceReducer: wrapReplaceReducer(store, deferEffects)
};
};
60 changes: 14 additions & 46 deletions src/enhanceReducer.js
View file
Original file line number Diff line number Diff line change
@@ -1,85 +1,53 @@
import AppStateWithEffects from './AppStateWithEffects';
import {
isFunction,
isUndefined,
isIterable,
invariant,
first,
mapIterable
} from './utils';

/**
* AppStateOrEffect mapper - maps Iterable to AppStateOrEffect data structure.
* The last element in Iterable chain is Application State the rest are some side effect.
* Iterator mapper, maps iterator to value
*
* @param {any} Side effect Function or Application State any
*
* @param {Boolean} Last element in iterable
* @returns {Object} AppStateOrEffect - Object containing isEffect(Boolean) and value(Function|Any)
*/
const mapIterableToEffectsAndAppState = (value, last) => ({
isEffect: !last,
value
});

/**
* AppStateOrEffect mapper - extracts just the value.
*
* @param {Object} Either Application State or Effect
* @returns {any} Might be either Function (for Effect) or any for Application State
*/
const mapValue = appStateOrEffect => appStateOrEffect.value;

/**
* Predicate function - decides whether appStateOrEffect is an Effect.
*
* @param {Object} Either Application State or Effect
* @returns {Boolean}
*/
const effectsPredicate = appStateOrEffect => appStateOrEffect.isEffect;

/**
* Predicate function - decides whether appStateOrEffect is an Application State.
*
* @param {Object} Either Application State or Effect
* @returns {Boolean}
* @param {Object} Iterator
* @returns {Any} Iterator Value
*/
const appStatePredicate = appStateOrEffect => !appStateOrEffect.isEffect;
const mapValue = iterator => iterator.value;

/**
* Reducer enhancer. Iterates over generator like reducer reduction and accumulates
* all the side effects and reduced application state.
*
* @param {Function} Root reducer in form of generator function
* @returns {Function} Reducer which returns AppStateWithEffects instead of simple Application state
* @param {Function} Function used for deferring effects accumulated from Reduction
* @returns {Function} Reducer
*/
export default rootReducer => (stateWithEffects, action) => {
export default (rootReducer, deferEffects) => (appState, action) => {
invariant(isFunction(rootReducer),
`Provided root reducer is not a function.`);

// Calling the Root reducer should return an Iterable
const reduction = rootReducer(stateWithEffects.getAppState(), action);
const reduction = rootReducer(appState, action);

if (isIterable(reduction)) {
// Iterable returned by Root reducer is mapped into array of values.
// Last value in the array is reduced application state all the other values
// are just side effects.
const appStateAndEffects = mapIterable(reduction, mapIterableToEffectsAndAppState);
const effects = mapIterable(reduction, mapValue);
const newState = effects.pop();

// It's necessary to separate array of effects and Application state from flat array
const effects = appStateAndEffects.filter(effectsPredicate).map(mapValue);
const appState = appStateAndEffects.filter(appStatePredicate).map(mapValue);
deferEffects(effects);

invariant(!isUndefined(first(appState)),
invariant(!isUndefined(newState),
`Root reducer does not return new application state. Undefined is returned`);

return new AppStateWithEffects(first(appState), effects);
return newState;
} else {
console.warn(
'createEffectCapableStore enhancer from redux-side-effects package is used, ' +
'yet the provided root reducer is not a generator function'
);

return new AppStateWithEffects(reduction, []);
return reduction;
}
};
18 changes: 1 addition & 17 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const mapIterable = (iterable, mapper) => {
// return the last value in iteration loop
const recur = acc => {
const next = iterable.next();
acc.push(mapper(next.value, next.done));
acc.push(mapper(next));

// ES6 tail call
return next.done ? acc : recur(acc);
Expand All @@ -94,22 +94,6 @@ export const mapIterable = (iterable, mapper) => {
return recur([]);
};

/**
* Returns first element in a non-empty array;
*
* @param {Array}
* @returns {any} First element in the provided Array
*/
export const first = arr => {
invariant(Array.isArray(arr),
`Provided argument is not array`);

invariant(arr.length > 0,
`Provided array is empty`);

return arr[0];
};

/**
* Standard map implementation which works with type of Object
*
Expand Down
21 changes: 0 additions & 21 deletions test/appStateWithEffects.test.js
View file

This file was deleted.

1 change: 0 additions & 1 deletion test/createEffectCapableStore.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { assert } from 'chai';
import { stub, spy } from 'sinon';
import { createStore } from 'redux';

import AppStateWithEffects from '../src/AppStateWithEffects';
import createEffectCapableStore, { wrapGetState, wrapDispatch } from '../src/createEffectCapableStore';

describe('Create effect capable store', () => {
Expand Down
1 change: 0 additions & 1 deletion test/enhanceReducer.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { assert } from 'chai';

import enhanceReducer from '../src/enhanceReducer';
import AppStateWithEffects from '../src/AppStateWithEffects';

describe('Enhance Reducer', () => {
it('should throw an exception when root reducer is not a function', () => {
Expand Down
21 changes: 1 addition & 20 deletions test/utils.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,6 @@ describe('Utils', () => {
assert.isFalse(Utils.isIterable(false));
});

it('should throw an exception when passing an empty array as an argument to `first` function', () => {
try {
spy(Utils, 'first');

Utils.first([]);
} catch (ex) {
assert.isTrue(Utils.first.threw());
}
});

it('should throw an exception when passing non array as an argument to `first` function', () => {
try {
spy(Utils, 'first');

Utils.first(false);
} catch (ex) {
assert.isTrue(Utils.first.threw());
}
});

it('should call the mapper function as many times as there are elements in the iterable', () => {
const mapper = spy(() => {});
Expand All @@ -81,7 +62,7 @@ describe('Utils', () => {
});

it('should return the mapped array', () => {
const mapper = val => val + 1;
const mapper = val => val.value + 1;

const mapped = Utils.mapIterable(generator(), mapper);

Expand Down

0 comments on commit 875ddc6

Please sign in to comment.