redux-data-structures icon indicating copy to clipboard operation
redux-data-structures copied to clipboard

Feature: "add all" action types for maps

Open bsonntag opened this issue 7 years ago • 5 comments

Right now there is no simple way to add multiple items at once to a map. We have to dispatch an add action for each element when we want to add a list of items.

It would be nice to have a addAllActionTypes option that would iterate on the action's payload and add all items. This could also need a collectionGetter option.

For lists, since you're using Array.prototype.concat, the add actions work for adding both single and multiple items.

bsonntag avatar Feb 02 '18 11:02 bsonntag

I see your point. Like you said, it's possible to add hundreds of items to a map at once by dispatching many actions. That's exactly what I do. I know it is not the most efficient way, but the performance impact has been negligible in my case, and a little for-loop in the client code isn't a big deal for me. It's kept the map API simple.

Now, I could see a situation where you already have an action with several items, which is reduced by another reducer, and you'd like to reduce that same action into a map, without having to dispatch new actions. Are you in that situation?

I can't promise that I'll implement this soon, so feel free to submit a PR. It would be greatly appreciated. I'd call it addMany... rather than addAll..., and I would also include a changeMany... and removeMany.... Also, how would you get the keys of the collection items?

Thank you for using redux-data-structures!

adrienjt avatar Feb 03 '18 04:02 adrienjt

Well, I'm using redux-promise-middleware for API requests. I get a list of entities on the fulfilled action that I want to add to a map. I still want to dispatch the fulfilled action because of some request state reducers (isFetching and stuff like that) and it would be nice if that would be enough to add all entities to the map.

Also, how would you get the keys of the collection items?

I was thinking that keyGetter would help with that, but its default is action => action.payload.id, so that won't work. We'd need collectionKeyGetter (item => item.id by default) and maybe even collectionItemGetter (item => ({ ...item }) by default).

Example:

const users = map({
  addActionTypes: ['CREATE_USER_FULFILLED'],
  addManyActionTypes: ['FETCH_USERS_FULFILLED'],
  changeActionTypes: ['UPDATE_USER_FULFILLED'],
  removeActionTypes: ['DELETE_USER_FULFILLED'],
  collectionGetter: action => [...action.payload], // default
  collectionItemGetter: item => ({ ...item }), // default
  collectionKeyGetter: item => item.id, // default
  itemGetter: action => ({ ...action.payload }), // default
  itemModifier: (item, action) => ({ ...item, ...action.payload }), // default
  keyGetter: action => action.payload.id, // default
});

I'll submit a PR for this when I have time.

bsonntag avatar Feb 05 '18 11:02 bsonntag

@adrienjt changeMany... isn't as easy as I first thought, because itemModifier takes the action as it's second argument. I'll have to add another option for modifying items from a collection.

What do you think about changing itemModifier to receive the result of itemGetter instead of action? Like this: itemModifier = (item, newItem) => ({ ...item, ...newItem }). This way I could easily implement changeMany... without adding another option. This would be a breaking change, though...

bsonntag avatar Feb 05 '18 21:02 bsonntag

Let's create an itemsModifier (note the plural) with your proposed signature for now, and I'll merge it with itemModifier in 1.0.0.

adrienjt avatar Feb 07 '18 23:02 adrienjt

I had the same issue: I solved it by using the reduce-reducers library to create a flat-combined reducer with the map reducer and a custom reducer for multiple add/remove/replace handling.

Code

import { map } from "redux-data-structures";
import reduceReducers from "reduce-reducers";

const reducer = reduceReducers(map({...}), replaceAllReducer, addAllReducer, removeAllReducer);

By doing this, the "all" reducers inherit the state shape from map, obtaining the visibility they need to operate on byId and allIds.

It's a workaround, but it works for the time being.

rsayn avatar Sep 06 '18 13:09 rsayn