Skip to content
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.

Sign up for GitHub

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

subscribe to throw when listener is not a function #1325

Merged
merged 5 commits into from
Jan 30, 2016
Merged

subscribe to throw when listener is not a function #1325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d575747
Added check to createStore.subscribe to ensure listener is a function
JamieDixon Jan 30, 2016
40fd771
Replaced error message with correct arg name
JamieDixon Jan 30, 2016
0c51819
Removed semi-colon from end of toThrow()
JamieDixon Jan 30, 2016
5279a93
Added tests for more deta types passed to createStore.subscribe
JamieDixon Jan 30, 2016
69f8d97
Removed extra new line after last assertion
JamieDixon Jan 30, 2016
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
4 changes: 4 additions & 0 deletions src/createStore.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export default function createStore(reducer, initialState, enhancer) {
* @returns {Function} A function to remove this change listener.
*/
function subscribe(listener) {
if (typeof listener !== 'function') {
throw new Error('Expected listener to be a function.')
}

listeners.push(listener)
var isSubscribed = true

Expand Down
20 changes: 20 additions & 0 deletions test/createStore.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,4 +548,24 @@ describe('createStore', () => {
store.replaceReducer(() => {})
).toNotThrow()
})

it('throws if listener is not a function', () => {
const store = createStore(reducers.todos)

expect(() =>
store.subscribe()
).toThrow()
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's check a few more data types: null, undefined, and string should throw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a few extra checks for the types mentioned. I've gone with the assertion style I've seen elsewhere rather than something like:

const cases = [undefined, null, ''];
    cases.map(arg =>
      expect(() =>
        store.subscribe(arg))
      .toThrow())


expect(() =>
store.subscribe('')
).toThrow()

expect(() =>
store.subscribe(null)
).toThrow()

expect(() =>
store.subscribe(undefined)
).toThrow()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: please remove the extra newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 removed in latest commit.

})
})