Skip to content

Remove FSA check in handleAction(s) #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@
"eslint-config-airbnb-base": "^1.0.3",
"eslint-plugin-import": "^1.5.0",
"eslint-watch": "^2.1.13",
"flux-standard-action": "^1.0.0",
"mocha": "^2.2.5",
"rimraf": "^2.5.3",
"webpack": "^1.13.1"
},
"dependencies": {
"flux-standard-action": "^1.0.0",
"invariant": "^2.2.1",
"lodash": "^4.13.1",
"reduce-reducers": "^0.1.0"
Expand Down
47 changes: 14 additions & 33 deletions src/__tests__/handleAction-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { expect } from 'chai';
import identity from 'lodash/identity';
import { handleAction, createAction, createActions, combineActions } from '../';

describe('handleAction()', () => {
Expand Down Expand Up @@ -96,6 +95,20 @@ describe('handleAction()', () => {
counter: 7
});
});

it('should not throw and return state when action is non-FSA', () => {
const reducer = handleAction(type, (state) => state, defaultState);
const action = {
foo: {
bar: 'baz'
}
};

expect(reducer(undefined, action)).not.to.throw;
expect(reducer(undefined, action)).to.deep.equal({
counter: 0
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chai assertion that we expect this code block not to throw? I think that's the point, and not that the state is reduced to a certain value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reducer should not throw AND just return the state. Will change that.

});
});
});

Expand Down Expand Up @@ -239,36 +252,4 @@ describe('handleAction()', () => {
.to.deep.equal({ number: 3 });
});
});

describe('with invalid actions', () => {
it('should throw a descriptive error when the action object is missing', () => {
const reducer = handleAction(createAction('ACTION_1'), identity, {});
expect(
() => reducer(undefined)
).to.throw(
Error,
'The FSA spec mandates an action object with a type. Try using the createAction(s) method.'
);
});

it('should throw a descriptive error when the action type is missing', () => {
const reducer = handleAction(createAction('ACTION_1'), identity, {});
expect(
() => reducer(undefined, {})
).to.throw(
Error,
'The FSA spec mandates an action object with a type. Try using the createAction(s) method.'
);
});

it('should throw a descriptive error when the action type is not a string or symbol', () => {
const reducer = handleAction(createAction('ACTION_1'), identity, {});
expect(
() => reducer(undefined, { type: false })
).to.throw(
Error,
'The FSA spec mandates an action object with a type. Try using the createAction(s) method.'
);
});
});
});
9 changes: 2 additions & 7 deletions src/handleAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import isNil from 'lodash/isNil';
import isUndefined from 'lodash/isUndefined';
import includes from 'lodash/includes';
import invariant from 'invariant';
import { isFSA } from 'flux-standard-action';
import { ACTION_TYPE_DELIMITER } from './combineActions';

export default function handleAction(actionType, reducer = identity, defaultState) {
Expand All @@ -24,12 +23,8 @@ export default function handleAction(actionType, reducer = identity, defaultStat
: [reducer.next, reducer.throw].map(aReducer => (isNil(aReducer) ? identity : aReducer));

return (state = defaultState, action) => {
invariant(
isFSA(action),
'The FSA spec mandates an action object with a type. Try using the createAction(s) method.'
);

if (!includes(actionTypes, action.type.toString())) {
const { type } = action;
if (type && !includes(actionTypes, type.toString())) {
return state;
}

Expand Down