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

Add pathvars & body in the store #152

Merged
merged 4 commits into from
Jul 10, 2017
Merged

Add pathvars & body in the store #152

merged 4 commits into from
Jul 10, 2017

Conversation

xurei
Copy link
Contributor

@xurei xurei commented Jul 10, 2017

It would be useful to have the path vars and/or the body of the request in the store. I personnaly use those information to detect if the data needs to be fetched again.

Here is my proposal for such feature.

@xurei xurei changed the title Added pathvars & body in the store Add pathvars & body in the store Jul 10, 2017
@lexich
Copy link
Owner

lexich commented Jul 10, 2017

Hi @xurei thx for you PR. I need time to test you solution, but I like your proposal. 👍

@lexich lexich merged commit 823e101 into lexich:master Jul 10, 2017
@lexich
Copy link
Owner

lexich commented Jul 10, 2017

@xurei you code in 0.10.7
Good job

@xurei
Copy link
Contributor Author

xurei commented Jul 11, 2017

Thanks :-)

Just thought about something though. If you bind multiple actions to the same store, that might create some weird behaviour... Any idea how we could improve the implementation for such cases ?

BTW, I made a react component based on your package : redux-api-react-switch
Any feedback about it ?

@lexich
Copy link
Owner

lexich commented Jul 12, 2017

@xurei Can you write example of weird behaviour?

redux-api-react-switch is very interesting project 👍

@xurei
Copy link
Contributor Author

xurei commented Jul 12, 2017

I'll create an issue with the details ;-)
Here it is : #153

Copy link
Contributor

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Ultimately the tests are constructed wrong. The actual actions are not dispatched in the format you specified in the tests.

I don't know the codebase good enough to tell if you could or should import the actual action creators or not.

I fixed it to work with the provided action creators in my local copy. I would start working on a PR if this is indeed currently bugged with the provided action creators

@@ -14,21 +14,20 @@ export default function reducerFn(initialState, actions={}, reducer) {
const { actionFetch, actionSuccess, actionFail,
actionReset, actionCache, actionAbort } = actions;
return (state=initialState, action)=> {
const params = action.params || {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Params are stored in action.request.params. This is currently always defaulting to {}

src/reducerFn.js Outdated
switch (action.type) {
case actionFetch:
return {
...state,
pathvars:{},
body:{},
pathvars: action.pathvars || {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Pathvars are stored in action.request.pathvars. This is currently defaulting.

@lexich
Copy link
Owner

lexich commented Jul 28, 2017

Hi @eps1lon I'll be glad to see new PR with bug fixing 👍

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.

None yet

3 participants