Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

add defaultState from the original reducers to the namespaced ones #100

Merged
merged 5 commits into from
Dec 13, 2018

Conversation

NikitaKolokoltsev
Copy link
Contributor

@NikitaKolokoltsev NikitaKolokoltsev commented Dec 11, 2018

Q A
Fixed Issues? Fixes #99
Patch: Bug Fix? 👍
Tests Added + Pass? Yes

Sorry for the mess in commits :)


const reduxSpecific = (action) => action.type.startsWith(REDUX_PREFIX)

export default reduxSpecific

Choose a reason for hiding this comment

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

add correct file ending

@@ -0,0 +1,13 @@
/**
* Copyright 2017, IOOF Holdings Limited.

Choose a reason for hiding this comment

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

Suggested change
* Copyright 2017, IOOF Holdings Limited.
* Copyright 2018, IOOF Holdings Limited.

Copy link
Contributor

@mpeyper mpeyper Dec 12, 2018

Choose a reason for hiding this comment

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

Github wont let me commit this (no permission to push the change, presumable to your fork), but yes.

@mpeyper mpeyper self-assigned this Dec 12, 2018

const processAction = (namespace) => (action, callback, defaultValue) => {
if (!namespace || isGlobal(action)) {
if (!namespace || isGlobal(action) || isReduxSpecific(action)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be tempted to move this into the isGlobal check. Essentially we are saying that redux actions are always global actions.


const processAction = (namespace) => (action, callback, defaultValue) => {
if (!namespace || isGlobal(action)) {
if (!namespace || isGlobal(action) || isReduxSpecific(action)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This case should get a specific unit test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add tests a bit later

@mpeyper
Copy link
Contributor

mpeyper commented Dec 12, 2018

Oh, I forgot to do the usual preamble...

Thanks for taking the time to improve redux-subspace. Don't forget to add yourself as a contributor before you're done!

@mpeyper mpeyper merged commit ac618ea into ioof-holdings:master Dec 13, 2018
@mpeyper
Copy link
Contributor

mpeyper commented Dec 13, 2018

Thanks for this. I'll put together a release later today.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using a prefilled state with namespaced reducers causes loss of the default store values.
3 participants