Versent / redux-crud

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

Why must current state be an array in reducers? #15

Closed fractaledmind closed 8 years ago

fractaledmind commented 8 years ago

I've started using redux-crud in a new application we're developing, but I've hit a problem when using it with Redux a number of times. I will get the error that the current state is not an array. My state tree is built primarily by using the Normalizr library, so the entity branches are dictionaries, where the keys are the ids.

I've read thru the source, but I'm still a bit unsure as to why current state not being an array will exit the reducer. Thoughts, suggestions?

sporto commented 8 years ago

At the moment redux-crud can only handle arrays. We did this because arrays can preserve order and that was important for us. Also unless you have thousands of items in a collection, arrays are quite fast for lookups.

I suggest converting your input collections to arrays instead of maps before sending them to the reducers.

fractaledmind commented 8 years ago

Have you looked at Normalizr? You suggest normalizing/flattening API responses in the ReadMe, but you do it manually. Normalizr maps allow for associations, e.g.

users: {
  1: {
    id: 1,
    name: 'Stephen Margheim',
    posts: [10, 11]
  }
posts: {
  10: {
    id: 10,
    title: 'My GitHub post'
  }

Why specifically does order preservation matter so much? Is there any way that this library could handle both arrays and objects (like seamless-immutable itself)? I'd be willing to work on a PR if so.

sporto commented 8 years ago

@smargh yes well looked at normalizr, didn't fit our way of organising data.

Definitely redux-crud could support handle collections as map. It is just a decent amount of work, happy to accept a PR on this if you are inclined to do it.

We did try using maps as collection at the very beginning, it is great for lookups, but we wanted to preserve the order of things as they come from the server, so we moved to arrays.

twmills commented 8 years ago

Regarding returning arrays from reducers...

I am trying to consume a JSON API endpoint and would like to include metadata such as the current page, or the sort direction and sort attribute. Forcing an array to be returned seems to prevent me from including this type of information for the collection.

Do you have a recommendation for how to handle that type of meta data?

sporto commented 8 years ago

@twmills maybe send that metadata through a different action to a reducer that just stores that information is a special object in your tree

farukg commented 8 years ago

i just started playing with redux-crud, but im also using normalizr :) I wonder in which cases the order from the server might be that important? In my case updates of specific items often cause a recalculation of the orderin. I wouldnt try to pick up the whole list from the server to reorder things, instead i'd rather reorder on client-side.

sporto commented 8 years ago

@farukg I agree that there are good arguments for storing records using a map. It is a trade off. We made a decision at some point which doesn't work well for everyone. We will be happy to receive a PR that implements an alternative storing model mapping by ids, if someone wants to undertake that.

jsherbert commented 8 years ago

I'm working on a fork which handles maps keyed by id when useMap is passed as a configuration parameter - if it turns out that this is a good way forward, I'll submit a PR when it's done.