Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing props in a composed selector's transform function #287

Closed
L-u-k-e opened this issue Sep 26, 2017 · 13 comments
Closed

Accessing props in a composed selector's transform function #287

L-u-k-e opened this issue Sep 26, 2017 · 13 comments

Comments

@L-u-k-e
Copy link

L-u-k-e commented Sep 26, 2017

Hopefully someone can help me here. I can't seem to find any other issues posing this exact question. (Perhaps that means my approach is wrong).

Let's say my app needs to render a list of todo items, and each TodoItem can be either expanded or collapsed. So, I have the following data structure in my redux store

{
   todoItemsExpansionMap: {
      [todoItemID]: false / true
      ...
   }
}

My JSX might look like:

TodoItemList.jsx

...
<div>
  {TodoItems.map(todo =>
     <TodoItem id={todo.id} key={todo.id} />
  )}
</div>

TodoItem.jsx

function TodoItem(props) { ... }

export default connect(
   (state, ownProps) => {
      // this is where I need help. I want to set an isExpanded prop based on the store using the value of the id prop that got passed down.
      isExpanded: ?????
   }
)(TodoItem

Right now, my selectors file looks like this:

export const getTodoItemExpansionMap = (state) => state.todoItemsExpansionMap;

export const makeIsTodoItemExpandedSelector = (instructionID) => createSelector(
  [getTodoItemExpansionMap],
  (expansionMap) => expansionMap[instructionID]
);

So, you can see that I'm using a factory function to generate a new selector for each ID that we will need to look at. This means my connect function would look like this:

   connect(
      (state, ownProps) => ({
         isExpanded: makeIsTodoItemExpandedSelector(ownProps.id)(state)
      })
   )

If my logic is correct, however, this won't memoize correctly because each time the mapStateToProps function is called, a branch new selector will be generated via the factory.

What am I missing here?

What I really want is to write the isTodoItemExpanded selector, not like a factory, but instead like so:

  export const isTodoItemExpandedSelector = createSelector(
    [getTodoItemExpansionMap],
    (expansionMap, args) => expansionMap[args.instructionID]
  );

The above is not possible however, unless Im missing some section of the docs.

@markerikson
Copy link
Contributor

You can use the "factory function" form of mapState to create a unique selector instance per component instance. I wrote an example as a Reddit comment a while back.

@L-u-k-e
Copy link
Author

L-u-k-e commented Sep 26, 2017

@markerikson So i'm still not sure the factory function form of mapStateToProps helps me.

I need to access my argument, not in the input selectors, but rather in the transform function. How can I achieve that? Can you maybe post a code snippet solving the issue above?

@markerikson
Copy link
Contributor

markerikson commented Sep 26, 2017

Try something like:

const mapStateFactory = (originalState, originalOwnProps) => {

    const isTodoItemExpandedSelector = createSelector(
        getTodoItemExpansionMap,
        (expansionMap) => expansionMap[originalOwnProps.instructionID]
    );

    return (state, ownProps) => {
        return {
            isExpanded : isTodoItemExpandedSelector(state)
        };
    }
}

You could also reuse that makeIsTodoItemExpandedSelector you have, and pass in originalOwnProps.instructionID as its parameter.

@L-u-k-e
Copy link
Author

L-u-k-e commented Sep 26, 2017

@markerikson Oh I'm starting to see, but I'm having trouble finding documentation on the factory syntax for the mapStateToProps function. I originally thought the function would receive no arguments, but now I see you've labeled 2 arguments, originalState and originalProps.

From this I think I can deduce that the factory receives the state and props the first time around and that on subsequent renders the function that the factory returns will be used on its own.

Is that correct? That would mean that the function wouldn't work right if the ID changes, right?

@markerikson
Copy link
Contributor

Yes, that example assumes the ID being passed in as a prop isn't changing, and yes, you've understood the behavior correctly.

A mapState function always receives at least one argument: the store state. You may declare a mapState function as taking two arguments, in which case connect will pass in the wrapper component's props as the second argument.

The factory function syntax is described in the API docs for connect:

Note: in advanced scenarios where you need more control over the rendering performance, mapStateToProps() can also return a function. In this case, that function will be used as mapStateToProps() for a particular component instance. This allows you to do per-instance memoization. You can refer to #279 and the tests it adds for more details. Most apps never need this.

Which, now that I look at it, is not all that informative. We should rewrite that.

@L-u-k-e
Copy link
Author

L-u-k-e commented Sep 27, 2017

@markerikson So, is there any way to make this work without relying on the assumption that the ID doesn't change?

@markerikson
Copy link
Contributor

markerikson commented Sep 27, 2017

You could change the selector so that it pulls in the current ID as an input:

const mapStateFactory = (originalState, originalOwnProps) => {

    const isTodoItemExpandedSelector = createSelector(
        [getTodoItemExpansionMap, (state, id) => id],
        (expansionMap, id) => expansionMap[id]
    );

    return (state, ownProps) => {
        return {
            isExpanded : isTodoItemExpandedSelector(state, ownProps.instructionID)
        };
    }
}

@Ice-Cream-Sandwiches
Copy link

Oh i see! That's the technique I had been missing.

So , more generally, if you need access to custom arguments in your transform function, you should make input selectors that return those arguments.

Thanks

@markerikson
Copy link
Contributor

Pretty much, yeah.

It's important to note that all of the "input selectors" you provide to createSelector will be called with the same arguments - the ones you passed into the actual selector. If your input selectors are expecting different arguments, then you'll have problems.

Here's a quick example:

const selectItems = state => state.items;
const selectId = (state, id) => id;
const selectName = (state, props) => props.name;

const selectItemInfo = createSelector(
    [selectItems, selectId, selectName],
    (items, id, name) => {
        const item = items[id];
        return {
            id,
            name,
            info : item.info
        };
    }
);

In this case, selectId expects that the second argument to selectItemInfo will be an ID value, and it just returns it. However, selectName expects that the second argument will be an object with a name field. If you call this selector as const itemInfo = selectItemInfo(state, 42), it will throw an error because 42.name is invalid.

I really need to turn that into a blog post :)

@Ice-Cream-Sandwiches
Copy link

Right so probably best practice is to pass an object containing named arguments and each input selector can deal with the ones it cares about and ignore the ones it doesn't care about.

@markerikson
Copy link
Contributor

Yep, I'd say that's probably a safe approach.

@markerikson
Copy link
Contributor

I filed reduxjs/react-redux#797 to cover rewriting the docs to better explain factory functions. I don't have time to do that myself right now, but if you or someone else would like to contribute, please leave a comment in that issue!

@markerikson
Copy link
Contributor

I wrote a blog post that discusses proper use of selectors and the "factory function" syntax. If someone would like to help update the docs based on that, please let me know!

Idiomatic Redux: Using Reselect Selectors for Encapsulation and Performance.

@ellbee ellbee closed this as completed Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants