feathersjs-ecosystem / feathers-redux

Integrate Feathers with your Redux store
MIT License
114 stars 23 forks source link

Remove isSaving/isLoading and replace by isCreating/isRemoving... #38

Closed amaury1093 closed 7 years ago

amaury1093 commented 7 years ago

Proposal:

Motivation

I often do the following, in my React component, to do something when a resource has been loaded.

componentWillReceiveProps(nextProps) {
  if (this.props.feathersUsers.isLoading && !nextProps.feathersUsers.isLoading) {
    // Do something when feathers finishes loading users
  }
}

However I wish to differentiate the behavior after patching a user or removing a user

componentWillReceiveProps(nextProps) {
  if (this.props.feathersUsers.isSaving && !nextProps.feathersUsers.isSaving) {
    // How do we know if we patched, updated or removed a user?
  }
}

Benefits

eddyystop commented 7 years ago

I agree the repo needs work in this area. I won't be able to get around to it until sometime in Jan 2018 because of work on Feathers Buzzard.

Are you interested in making a PR?

bsubedi26 commented 7 years ago

@amaurymartiny, I actually had this problem also have this implemented in a locally forked repo for testing and its working well. I could definitely submit a PR. Its actually a few lines of code.

Depending on the action you dispatch for the respective service, the state is changed to (`${actionName}Pending). For example, a create action (createPending: true), find action (findPending: true) ... (etc.).

const reducerForServiceMethod = (actionType, ifLoading, isFind) => ({
const slicedActionType = actionType.slice(SERVICE_NAME.length, actionType.length).toLowerCase() // returns find/create/update/patch
...
})

When the promise has started returns the following state as always with the following addition:

return ({
        ...state,
        [opts[`${slicedActionType}Pending`]]: true,
       ...
})

When the promise has resolved:

return ({
        ...state,
        [opts[`${slicedActionType}Pending`]]: false,
       ...
})

When promise is rejected:

return ({
        ...state,
        [opts[`${slicedActionType}Pending`]]: false,
})

That is all the code, you can look at this file for reference (https://github.com/bsubedi26/React-Node-Authentication/blob/master/src/lib/feathers-redux.js).

amaury1093 commented 7 years ago

Looks good! Yes go ahead and submit the PR

eddyystop commented 7 years ago

+1 I love not working in a vacumn.

bsubedi26 commented 7 years ago

Pull Request (https://github.com/feathers-plus/feathers-redux/pull/39).

eddyystop commented 7 years ago

I asked on the PR is the failing integration tests could be fixed.

amaury1093 commented 6 years ago

Would it make sense to remove isSaving/isLoading after #39?

eddyystop commented 6 years ago

Quite possibly. Such a change should have a major semver change.

I plan to get my mind back into this repo in Feb once I finish some other Feathers work.