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.

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

Ability to run a different reducer instead of returning the a static initialState #5

Merged
merged 8 commits into from
Jul 25, 2016
Merged

Ability to run a different reducer instead of returning the a static initialState #5

merged 8 commits into from
Jul 25, 2016

Conversation

arikmaor
Copy link
Contributor

I think it can be a common case for example:

function reducer(state = { a: 'should keep its value', b: 0 }, action) {
  switch(action.type) {
    case 'SET_A':
      return {
        ...state,
        a: 'SET!'
      }
    case 'SET_B':
      return {
        ...state,
        b: state.b+1
      }
    default:
      return state
  }
}

function reset(state, action) {
  return {
    ...state
    b: 0
  }
}

@arikmaor
Copy link
Contributor Author

I've explaind the change in the README 73e23c6

root = true

[*]
indent_style = tab
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove this file from the PR? When I squash and merge we'd have two separate changes in one comment. Also, I used spaces across the project 😅

Feel free to create a new PR for .editorconfig

@omnidan
Copy link
Owner

omnidan commented Jul 25, 2016

Thanks a lot - this definitely looks interesting! I've added some comments to the files 😉

@@ -49,6 +50,19 @@ combineReducers({

Now, once you click the increment button, the state will be reset to `0`.

if you need more complex initialization logic you can provide a `reducer function` as the last param it willadd be used to get the inital state.
Copy link
Owner

Choose a reason for hiding this comment

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

Small changes: If you need more complex initialization logic, you can provide a reducer function as the last param. It will be called with the state and action to get the initial state.

@omnidan
Copy link
Owner

omnidan commented Jul 25, 2016

@arikmaor I've added a comment for a minor text improvement, once it's fixed this should be ready 😄

@omnidan
Copy link
Owner

omnidan commented Jul 25, 2016

@arikmaor sorry to bother you again, but can you pin eslint to ~2.2.0 to work around the issue with babel-eslint and make CI pass?

@arikmaor
Copy link
Contributor Author

lint still fails with this output:

/Users/arik/Projects/redux-recycle/src/index.js
  2:1  error  Definition for rule 'no-duplicate-imports' was not found     no-duplicate-imports
  2:1  error  Definition for rule 'no-unsafe-finally' was not found        no-unsafe-finally
  2:1  error  Definition for rule 'no-useless-computed-key' was not found  no-useless-computed-key
  2:1  error  Definition for rule 'no-useless-escape' was not found        no-useless-escape

/Users/arik/Projects/redux-recycle/test/index.spec.js
  1:1  error  Definition for rule 'no-duplicate-imports' was not found     no-duplicate-imports
  1:1  error  Definition for rule 'no-unsafe-finally' was not found        no-unsafe-finally
  1:1  error  Definition for rule 'no-useless-computed-key' was not found  no-useless-computed-key
  1:1  error  Definition for rule 'no-useless-escape' was not found        no-useless-escape

@omnidan
Copy link
Owner

omnidan commented Jul 25, 2016

@arikmaor Thanks for all your efforts! I cloned your repo and managed to get it to work by also pinning the eslint-config-standard package to ~5.1.0. Now npm run lint works fine for me. Can you try that? 😁

@arikmaor
Copy link
Contributor Author

seems to work, thanks :)

@omnidan omnidan merged commit 59a101b into omnidan:master Jul 25, 2016
@omnidan
Copy link
Owner

omnidan commented Jul 25, 2016

Thank you, @arikmaor - merged and released 1.2!

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

Successfully merging this pull request may close these issues.

2 participants