reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.88k stars 15.27k forks source link

Data abstraction #951

Closed jaroslawr closed 9 years ago

jaroslawr commented 9 years ago

Have you at any point considered introducing more data abstraction into Redux? I think the lack of it becomes an issue as you write more complicated, real-world code. All Redux examples I have seen do something like this:

function todos(state = [], action) {
  switch (action.type) {
  case ADD_TODO:
    return [...state, {
      text: action.text,
      completed: false
    }];
  case COMPLETE_TODO:
    return [
      ...state.slice(0, action.index),
      Object.assign({}, state[action.index], {
        completed: true
      }),
      ...state.slice(action.index + 1)
    ];
  default:
    return state;
  }
}

Many times the code inside several different case branches is very similar except for one value or one property being different, e.g. soon you end up having:

switch (action.type) {
  case COMPLETE_TODO:
    return [
      ...state.slice(0, action.index),
      Object.assign({}, state[action.index], {
        completed: true
      }),
      ...state.slice(action.index + 1)
    ];
  case RENAME_TODO:
    return [
      ...state.slice(0, action.index),
      Object.assign({}, state[action.index], {
        text: action.text
      }),
      ...state.slice(action.index + 1)
    ];
  default:
    return state;
}

Wouldn't it be cleaner to have an abstract data type defined, like you ordinarily would do in a functional language, and then have the actions just execute operations on the abstract data type? E.g.:

function makeTodo(text, completed) {
  return {
    text: text,
    completed: completed
  };
}

function completeTodo(todo) {
  Object.assign({}, todo, {
    completed: true
  });
}

function renameTodo(todo, text) {
  Object.assign({}, todo, {
    text: text
  });
}

function makeTodos() {
  return [];
}

function addTodo(todos, text, completed) {
  return [...todos, makeTodo(text, completed)];
}

function modifyTodo(todos, index, modify) {
  return [
    ...todos.slice(0, index),
    modify(todos[index]),
    ...todos.slice(index + 1)
  ];
}

And then:

function todos(state = makeTodos(), action) {
  switch (action.type) {
  case ADD_TODO:
    return addTodo(state, action.text, false);
  case COMPLETE_TODO:
    return modifyTodo(state, action.index, (todo) => completeTodo(todo))
  case RENAME_TODO:
    return modifyTodo(state, action.index, (todo) => renameTodo(todo))
  default:
    return state;
  }
}

For anything non-trivial I think the second style is much cleaner and has all the usual advantages abstract data types have over direct modification of data:

https://en.wikipedia.org/wiki/Abstract_data_type#Advantages_of_abstract_data_typing

It's clear you can already do this with Redux, but the docs in their current form perhaps encourage the unstructured version too much. There doesn't even seem to be a place in most projects for those things. Maybe it would be wise to have something like models/ (e.g. todo.js with makeTodo and completeTodo) and collections/ (e.g. todos.js with makeTodos and modifyTodo) in Redux apps. Then you can reuse those things in various different reducers. This seems like an easy clear improvement to me, nevertheless I haven't seen this done in any of the open source Redux apps I have seen around.

jaroslawr commented 9 years ago

Consider one or the other approach on the scale of this for example:

https://github.com/erikras/redux-form/blob/master/src/reducer.js

What if I now need to change shape of the the whole tree slightly? It becomes a nightmare to maintain if you don't express it all in terms of a set of abstract higher level operations.

gvergnaud commented 9 years ago

To avoid this scale problem I've came up with some helper methods to make your reducers more declarative using composition of mutations. Here is an example where you fetch videos from some API.

const initialState = {
  isFetching: false,
  hasApiError: false,
  ids: [],
  videosById: {}
}

let setIsFetching = (isFetching) => () => ({
  isFetching
})

let setHasApiError = (hasApiError) => () => ({
  hasApiError
})

let addVideos = (state, { result, entities }) => ({
  ids: [
    ...new Set([
      ...state.ids,
      ...(Array.isArray(result) ? result : [result])
    ])
  ],
  videosById: {
    ...state.videosById,
    ...entities.video
  }
})

let deleteVideos = (state, { result }) => ({
  ids: state.ids.filter(id =>
    Array.isArray(result) ?
      !result.some(resultId => id === resultId) : id !== result
  ),
  videosById: pick(state.videosById, (id, video) =>
    Array.isArray(result) ?
      !result.some(resultId => id === resultId) : id !== result
  )
})

export default createReducer(initialState, {
  [VIDEOS_FETCH_REQUEST]: compose(
    setIsFetching(true)
  ),

  [VIDEOS_FETCH_SUCCESS]: compose(
    setIsFetching(false),
    setHasApiError(false),
    addVideos
  ),

  [VIDEOS_FETCH_ERROR]: compose(
    setIsFetching(false),
    setHasApiError(true)
  ),

  [VIDEO_ADD_SUCCESS]: compose(
    addVideos
  ),

  [VIDEO_DELETE_SUCCESS]: compose(
    deleteVideos
  )
})

I am using a createReducer function i found there defined like this :

function createReducer(initialState, handlers) {
  return function reducer(state = initialState, action) {
    if (handlers.hasOwnProperty(action.type)) {
      return handlers[action.type](state, action);
    } else {
      return state;
    }
  }
}

so first i define my initialState object, and the implementations of the state mutations I am going to use. This mutations takes the current state as first argument and the action as second. Then i compose my mutations with this compose function :

export let compose = (...mutations) => (state, action) =>
  mutations.reduce(
    (mutatedState, mutation) => ({ ...mutatedState, ...mutation(mutatedState, action) }),
    state
  )

Note that this is not a 'real' compose function, and that it only works for an object defined state.

I like this approach because it helps me keeping my code DRY, by using several times my mutations instead of having a handler for each action.

gaearon commented 9 years ago

Feel free to contribute to the docs. I agree it's valuable to explain that you can call functions from other functions. People often forget this :-) There's only so much effort I could afford to spend on the documentation, and at this point it is entirely community-driven.

gaearon commented 9 years ago

If you work on this, please don't add models or collections—that would introduce the wrong idea that doing something like this is required.

Just adding a section in http://redux.js.org/docs/recipes/ReducingBoilerplate.html and at the end of http://redux.js.org/docs/basics/Reducers.html should be enough.