reduxjs / reselect

Selector library for Redux
MIT License
19.04k stars 670 forks source link

Question - 'Tree' of Selectors/Components Design Pattern #211

Closed Ryan-Vereque closed 7 years ago

Ryan-Vereque commented 7 years ago

Technologies discussed - reselect, React, React-redux

I have been playing around with https://github.com/mxstbr/react-boilerplate and have encountered a difficulty with using reselect when it comes to multiple components needing private copies of selectors dependent on their props.

The common approach I see being used is implementing factory functions that create these selectors, and then referencing those factory functions in the mapStateToProps function when connecting the component to the store. This approach is described in the reselect docs, https://github.com/reactjs/reselect/blob/master/README.md#sharing-selectors-with-props-across-multiple-components like so:

const getVisibilityFilter = (state, props) =>
  state.todoLists[props.listId].visibilityFilter

const getTodos = (state, props) =>
  state.todoLists[props.listId].todos

const makeGetVisibleTodos = () => {
  return createSelector(
    [ getVisibilityFilter, getTodos ],
    (visibilityFilter, todos) => {
      switch (visibilityFilter) {
        case 'SHOW_COMPLETED':
          return todos.filter(todo => todo.completed)
        case 'SHOW_ACTIVE':
          return todos.filter(todo => !todo.completed)
        default:
          return todos
      }
    }
  )
}
const makeMapStateToProps = () => {
  const getVisibleTodos = makeGetVisibleTodos()
  const mapStateToProps = (state, props) => {
    return {
      todos: getVisibleTodos(state, props)
    }
  }
  return mapStateToProps
}

My curiosity lies in the cases when I desire a group of sibling components to use private copies of a selector that itself is composed of selectors (via inputSelectors argument of createSelector) that should be dependent on props of the parent component, forming a tree phenomena. To illustrate let me describe an example.

Let's say we have an app that is similar to the todo list example specified in the docs above, that is to say it still keeps track of multiple todo lists with their own visibility filters and each being composed of an array of Todo components. TodoList is a component that receives its props.todos when it is connected to the store using the above makeMapStateToProps. TodoList shown here:

const TodoList = ({ todos, onTodoClick }) => (
  <ul>
    {todos.map(todo =>
      <Todo
        key={todo.id}
        {...todo}
        onClick={() => onTodoClick(todo.id)}
      />
    )}
  </ul>
)

TodoList.propTypes = {
  todos: PropTypes.arrayOf(PropTypes.shape({
    id: PropTypes.number.isRequired,
    completed: PropTypes.bool.isRequired,
    text: PropTypes.string.isRequired
  }).isRequired).isRequired,
  onTodoClick: PropTypes.func.isRequired
}

The difference with our new example is that Todo components will not be passed their props via their parent component TodoList's props, they will instead be components connected to the store independently. Each Todo component will also now require a private selector that accesses the store for its todo specific information (id,completed, and text).

The tree phenomena I refer to occurs because each of the private selectors for sibling Todos should ideally be composed using a selector that is common to the TodoList these Todos reside in, but not common to all TodoLists. The nesting of components align with nesting of information in the state. Therefor I believe (especially for larger apps) it would be more efficient to employ a design pattern that reflects this tree-like behavior (where nodes of this tree are not predefined but determined by user initiated actions).

I have some ideas on to go about this. Given that prop dependent selectors will usually occur in situation where a parent component's children components are all components of the same type, then one could leverage the fact that the parent component often creates the array of children and simply pass each child its own private copy of a selector, each composed with the same parent selector, but the outer selection being made with the child identifying information (like the ID of a single Todo). If the nesting has more layers then no problem, when creating the private copies of selectors for the children components, compose them using the selector that was passed down the parent component.

A problem I see with this however is that the creation of selectors will now be left up to components. (unless the get_____ functions may take other selectors as parameters to be used in the composing)

What do you all think of this ? Contrived ? Has anyone else solved this problem ? Any input would be greatly appreciated and I imagine this a problem that developers of larger apps using reselect have encountered.

EDIT IMPORTANT I now realize my state design was predicated on a non-normalized state. Please look into normalization if you are find yourself wanting to nest entries of data or relate different types of entries in any way!

heyimalex commented 7 years ago

Personally I feel like "memoizing tree-like derived data" is really React's domain. Why do you need reselect in this scenario?

Ryan-Vereque commented 7 years ago

I would still like to leverage reselect's main advantage, not having to find and retrieve a sub-portion of the state tree each time I need it, rather only when it has changed. React components could certainly retain the reselect selector that corresponds to the part of the tree they require, but the problem of creating that selector remains. As I explained, this requires a tree of private selectors if we are to avoid thrashing (a single selector selecting from different portions of the tree due it not being private) and redundancy (creating multiple private selectors for the same portion of a tree). This tree of selectors therefore mimics the form of state tree, except it may not have a selector for EVERY node of tree.

May I ask simply how you would solve the TodoList problem I specified above ? (where Todo components have their own selectors that is). Would you give all your Todo components the same selector and incur thrashing or would you give each of them a private selector that is composed from only other private selectors that become redundant? Or do you have some other approach (what I'm searching for) that avoids these issues and is also not completely in-elegant.

heyimalex commented 7 years ago

The difference with our new example is that Todo components will not be passed their props via their parent component TodoList's props, they will instead be components connected to the store independently.

I just don't understand why you need to connect the Todo components directly at all. This is what React is good at; pass in the props directly, extend from PureComponent or write shouldComponentUpdate and call it a day. As it is, you're trying to re-create React with reselect. I really doubt anything you could do would garner better performance. But I'd love to be proven wrong.

Anyways, you won't get around manually passing down the selector, but maybe this would work.

function makeTodoListMapStateToProps() {
  const metaListSelector = createSelector(
    (state, props) => props.listId,
    (listId) => createSelector(
      (state) => state.todoLists[listId].visibilityFilter,
      (state) => state.todoLists[listId].todos,
      (visibilityFilter, todos) => {
        switch (visibilityFilter) {
          case 'SHOW_COMPLETED':
            return todos.filter(todo => todo.completed)
          case 'SHOW_ACTIVE':
            return todos.filter(todo => !todo.completed)
          default:
            return todos
        }
      }
    ),
  );
  return function mapStateToProps(state, props) {
    const listSelector = metaListSelector(state, props);
    return {
      todos: listSelector(state, props),
      listSelector,
    }
  }
}

function makeTodoMapStateToProps() {
  const metaTodoSelector = createSelector(
    (state, props) => props.listSelector,
    (state, props) => props.todoId,
    (listSelector, todoId) => createSelector(
      listSelector,
      (list) => list[todoId],
    ),
  );
  return function mapStateToProps(state, props) {
    const todoSelector = metaTodoSelector(state, props);
    return {
      todo: todoSelector(state, props),
    }
  }
}
markerikson commented 7 years ago

@heyimalex : actually, connecting individual list items is usually a better performance pattern when using Redux and React together, so the question does make some sense. See http://redux.js.org/docs/faq/ReactRedux.html#react-multiple-components and http://redux.js.org/docs/faq/Performance.html#performance-scaling .

Also, only partly following the discussion here, but @kicknickr , have you tried using the "factory function" syntax for connect to allow you to create individual selector instances per component? I have an example at https://www.reddit.com/r/reactjs/comments/5dxasp/any_deepdiveadvanced_tutorials_on_reselect/da88r5m/?context=3 .

Ryan-Vereque commented 7 years ago

@markerikson : I'm quite aware of the approach shown in that reddit post, it's just a particular implementation of creating (what I've seen dubbed) 'private' selectors by using factory functions to creating them.

Again, my problem was moreso what do you when you have a more complex situation where selectors could/should be composed ? If you're app has a single TodoList with multiple Todo components, then sure one may simply use a factory function that create a private copies of a selector for each Todo that also uses composition (composed using a hypothetical TodoList selector). But when you have multiple TodoLists of course which TodoList selector used in the composition is relevant. If one were to use a single TodoList selector in the composition of all Todos across all TodoLists, then we would again run into the problem of thrashing or useless memoization. Thusly, we want the TodoList selectors to also be private and made with factory functions (and have the Todo selectors be composed using their appropriate TodoList selector).

In this way, the selector composition leads up to a tree phenomena where 'children' selectors are private and may be composed of selectors higher up in the tree that are also private. This coordination may become increasingly tricky and is already tricky with the example of TodoLists. Now imagine if we generically had that A components have any number of child B components who have any number of child C components and so on where this data structure is mirrored in the state tree ?

An approach I've thought of recently is perhaps one could maintain a single object that mirrors that state tree, but also holds any selector for at a given node of the tree. That is to say, such tree starts out empty, and any time a selector with a specified path down the tree is requested, it will travel down existing portions of the tree, and where the tree is not yet existing it will create what is missing by creating a node and traveling down that node until the full specified path through the tree has been traveled. When it creates a node, it actually creates a new object under a children field that contains both the corresponding selector for said node, and a field for children. That new selector will be composed using the selector of it's parent node (the whole benefit of this approach) and also be privately created using a factory function. An example tree might be as follows:

global: ----selector: ... ----children: --------ExtraAppStuff: ------------selector: ... ------------children: NULL --------TodoLists: ------------selector: ... ------------children: ----------------TodoList1: --------------------selector: ... --------------------children: ------------------------Todo1: ----------------------------selector: ... ----------------------------children: NULL ------------------------Todo2: ----------------------------selector: ... ----------------------------children: NULL ------------------------Todo3: ----------------------------selector: ... ----------------------------children: NULL ----------------TodoList2: --------------------selector: ... --------------------children: ------------------------Todo4: ----------------------------selector: ... ----------------------------children: NULL ----------------TodoList3: --------------------selector: ... --------------------children: ------------------------Todo5: ----------------------------selector: ... ----------------------------children: NULL ------------------------Todo6: ----------------------------selector: ... ----------------------------children: NULL

Another aspect of this design is that given Todo components may have conflicting IDs, it may be simpler to implement random alphanumeric strings as IDs for parts of the tree that are not known to exist beforehand or are not already guaranteed some uniqueness among it's siblings. React components will pass down their path (the path of the information they require in the state tree as a list of strings) to their children including a randomly generated ID for their child at the end of the path. A child now being aware of it's path in the state tree can now request from our maintained object to provide a selector that path using the mechanisms described above.

markerikson commented 7 years ago

Two thoughts:

Ryan-Vereque commented 7 years ago

I have a question about your suggestion, are you creating a private selector using full paths in the state tree for each [Todo] component ? This would mean you'd fail to compose Todo selectors using a common TodoList selector. I imagine when you have many layers to the tree, this creates an exponential amount of redundant selector data. Perhaps this is still very negligible (I don't know how much larger selector objects get when they use a full explicit path (a long one theoretically that is) in the tree vs using a single composition) ? Do you think it's negligible ?

markerikson commented 7 years ago

Maybe we're talking about a couple different things here. You keep talking about "Todo" selectors and "TodoList" selectors. What do you mean when you say a "list" selector? And for that matter, "redundant selector data"?

If I was making a selector for a given Todo list item component, it'd probably look roughly like this:

const selectEntities = state => state.entities;
const selectTodos = createSelector(selectEntities, entities => entities.Todo);

const makeTodoSelectorById = (id) => {
    const idSelector = () => id;

    return createSelector(
        selectTodos, idSelector,
        (todos, todoId) => todos[todoId]
    );
}

const makeTodoListItemMapState = (state, ownProps) => {
    const selectTodoForThisComponent = makeTodoSelectorById(ownProps.id);

    const actualMapState = (state, ownProps) => {
        const todo = selectTodoForThisComponent(state);

        return {todo};
    };
}

Totally off the top of my head, but that's the basic idea.

So, I'm really not following you when you're talking about "full explicit paths" and "redundant selector data".

markerikson commented 7 years ago

Like I said, I hadn't read through the entire discussion fully. Looking back at your original example, it looks like you only have your TodoLists connected, and not the TodoListItems themselves. That suggests that you don't have your state normalized. Normalized state is the standard suggestion for optimized performance and easy access to items by ID. If you're not familiar with that, you might want to read through http://redux.js.org/docs/faq/OrganizingState.html#organizing-state-nested-data and http://redux.js.org/docs/recipes/reducers/NormalizingStateShape.html .

Ryan-Vereque commented 7 years ago

I might be mistaken, but I think TodoListItem is what I've been referring to simply by Todo. I will refer to it that way from now on.

I was drawing from the example in the reselect documentation where many TodoLists each contain many TodoListItems (this sort of nesting is true in both the way it is represented in the state tree and the React components themselves). A 'x' selector refers to a selector for any portion of the state tree that theoretically represents a single 'x' and also has a corresponding React component that represents 'x'. By 'full explicit path' I meant to say that one would ignore composition and use a state.a.b.c.d.e... approach for every selector. By 'redundant selector data' I meant that making a selector using an explicit path without composition would mean that you'd have many selectors memorizing the same shared portions of the tree and so 1)require more memory 2) lose out of the key benefit of having all selector calls update internally share memorized information regarding a specific portion of the tree

I had not actually read about Normalization before, but upon quickly reviewing your links, I had those criteria in mind when designing my state.

Looking at your code, you seem to be implementing an app with a state that only maintains a single list of TodoListItems and creates selectors using factory functions a method I've been referred to several times and acknowledged, but criticized as it applies to more complex state trees. Have you considered how your implementation will scale when you introduce the concept of a TodoList and multiple at that ? Will you create a selector for a TodoList and that takes an ID for the TodoList as a parameter ? (similar to what you did for TodoListItems so far), and then use that in the composition for TodoListItem ? If so you will have to use a factory function to create a selector for each TodoList and then make sure TodoListItems use the appropriate one in their composition.

markerikson commented 7 years ago

Yeah, I really think you're over-thinking this, on a few levels :)

The point of normalization is that all items of a given type are in that lookup table. If I had 10 different TodoLists, each with their own set of 100 unique Todo entries, the actual Todo data items would all be stored under state.entities.Todo, and that object would contain all 1000 of them keyed by ID. No matter what list a Todo is considered part of, I can do a direct lookup of a specific Todo entry just by knowing its ID.

If I need to track what Todo entries are in a TodoList, I can store an array of IDs. So, similarly, I would have state.entities.TodoList (potentially), and each TodoList data item might look like {id : 1, todos : [4, 27, 98, 34, .....], visibilityFilter : "SHOW_ALL" }. From there, the corresponding TodoListComponent could filter that list as needed in its mapState function by mapping its list of todo IDs to their corresponding items, and filtering the result.

Seriously, I think almost your entire original question is predicated on the idea of having a non-normalized state. Once you normalize, a lot of this stuff becomes much simpler.

Ryan-Vereque commented 7 years ago

Oh you are completely right I did not look over your links carefully enough. Normalization is very much a good idea :smile: It was predicated on a non-normalized state, yup.

Thank you @markerikson for pointing this out to me haha.