Versent / redux-crud

A set of standard actions and reducers for Redux CRUD Applications
MIT License
624 stars 54 forks source link

Not mapping id to object properly #68

Closed dvreed77 closed 7 years ago

dvreed77 commented 7 years ago

My API delivers a JSON array that looks something like this

[{"id":22, "data": "d"}, {"id": 1, "data": "2"}]

I am using the Map Reducer and so I would expect the output to be:

{1: {"id": 1}, 22: {"id": 22}}

but instead I am getting the following

{0: {"id":22}, 1: {"id":1}}

Is this correct behavior?

Thank you

sporto commented 7 years ago

No, that looks broken. I will investigate this.

sporto commented 7 years ago

This seems to be working as expected. At least in the tests https://github.com/Versent/redux-crud/blob/master/src/reducers/map/fetch/success.test.ts#L27

Any more details you can give me?

Do you pass an additional config to reducersFor(). How does it look like?

dvreed77 commented 7 years ago

Hmm, it must have been my fault, but not sure what is happening. I was trying to do stuff manually in the reducer, and ended up just leaving it like this:

let regexpActions = reduxCrud.actionTypesFor('regexp');
const standardReducers = reduxCrud.Map.reducersFor('regexp', {key: 'id'})

function regexpReducers(state={}, action) {
    switch(action.type) {
        case regexpActions.fetchSuccess:
            return merge({}, state, action.records)
        default:
            // pass to the generated reducers
            return standardReducers(state, action);
    }
}

export default combineReducers({
    routing: routerReducer,
    regexp: regexpReducers,
})

Which was causing the issues I spoke about.

Changing it to this:

export default combineReducers({
    routing: routerReducer,
    regexp: reduxCrud.Map.reducersFor('regexp')
})

works as expected.

Not sure what the difference, but it works. Thanks for the quick reply though!

sporto commented 7 years ago

Hi.

In return merge({}, state, action.records) action.records should be an array. So it seems that your merge is using the indexes.

yasuf commented 7 years ago

should this be closed?

sporto commented 7 years ago

Yes closing, as this isn't a bug.