Skip to content

Redux Tests #7

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 16 commits into from
Jun 25, 2017
Merged

Redux Tests #7

merged 16 commits into from
Jun 25, 2017

Conversation

AllenFang
Copy link
Member

@AllenFang AllenFang commented Jun 13, 2017

Use redux-api-middleware instead of redux-thunk

  • reducers
  • actions
  • redux-api-middleware test

@AllenFang AllenFang added this to the 0.1.0-alpha.1 milestone Jun 13, 2017
@AllenFang AllenFang requested a review from CYBAI June 13, 2017 14:49
@AllenFang
Copy link
Member Author

AllenFang commented Jun 18, 2017

@CYBAI please check AllenFang@86d55e9 and AllenFang@0844659

I only have this two commit in my fork, and this both commits is for using fetch with redux-fetch-middleware instead of using fetch with redux-thunk.

Something need discuss:

  1. in the there, due to those fetch action is not async anymore, but action creator must return an object with type(This is rule by calling bindActionCreator). So I force return a no operation(aka, NO_OP) action. Actually, there's a middleware call redux-multi which can help us to return multiple action creator, but it most like out of maintain for a very long time and also have some bug(But I've give a try with ungit).

  2. redux-fetch-middleware is also seems like out of maintain.

  3. Using this middleware will force us to use store.dispatch in action, like here and here

Please let me know your concern, it's just a experiment and we dont must use it but if we want to use this, I can base this two commits to create another PR.

Thanks

@AllenFang
Copy link
Member Author

BTW, The test of reducers are done, please review.

@CYBAI
Copy link
Contributor

CYBAI commented Jun 18, 2017

@AllenFang

  1. About redux-fetch-middleware, due to out of maintain, I'm not sure if we can change to redux-api-middleware.
  2. I'll review this PR in these days; in the meantime, could you help to add .travis.yml to trigger the travis?

Thanks.

@AllenFang
Copy link
Member Author

It seems a good idea to use redux-fetch-middleware, but I think return multi actionCreator and using store.dispatch still can't be avoid? I need to check more but I guess this two issues still remain even we use redux-fetch-middleware.

Copy link
Contributor

@CYBAI CYBAI left a comment

Choose a reason for hiding this comment

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

Test part LGTM.
Could you help to add .travis.yml to trigger testing?

@AllenFang AllenFang changed the title [WIP] Redux Tests Redux Tests Jun 25, 2017
@CYBAI
Copy link
Contributor

CYBAI commented Jun 25, 2017

Merge this PR first, have to fix TypeError: Cannot read property 'async' of undefined later.

@CYBAI CYBAI merged commit 18ad19c into revamp-with-react:phase-1 Jun 25, 2017
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