1

This is a somewhat more style/theory question as I THINK both methods will work. Here is the scenario:

I have an InfiniteList component that I want to keep generic. It gets the current list of all item IDs from the parent, then figures out what ones it has to actually render. To do that, I basically pull the appropriate IDs from the list, then go fetch the full data by ID from the store.

My question is this: To keep it generic, obviously the Infinite List component can't really hardcode what store it should be getting the full item data from (say I have 10 different types of items that all have their own store). However, it makes more sense to me that scrolling around and changing the set of items being displayed is a state thing. So, do you think it makes sense to:

A) Pass the list of IDS as props, as well the add/remove listeners in from the parent component, so the list component knows who to listen to?

B) Or does it make more sense to just pass in both the list and the full set of item data available in as props and have the parent component listen to the appropriate store?

Part of the motivation behind "listening" is that if the store doesn't have the items, it must fetch them, so I need the list to rerender once the itemStore updates.

Inspired partly from here: ReactJS: Modeling Bi-Directional Infinite Scrolling

Community
  • 1
  • 1
SleepyProgrammer
  • 413
  • 1
  • 4
  • 15
  • 1
    I think it is better to have the controller as high as possible in the hierarchy, therefore to fetch the data from the InfiniteList rather than from each component. – gcedo Jun 04 '15 at 15:31

1 Answers1

2

I would go with B for the following reasons:

1) I think this would result in the most generic Component, because it doesn't need to know anything about where the data is coming from or how it is updated.

2) the Component has no state.

3) the code to listen for Store events and update the Component's state is likely so short you won't enjoy much significant benefit from moving it into the component.

The only advantage I'd see A having is moving that store event handling into the component, so you don't have to rewrite it. But by doing this, you'd force your component to have state. Maybe in this case, it's straightforward enough not matter. It really depends on the application. If components higher up in the hierarchy (parents of the InfiniteList) are also going to re-render on events from the same store, I'd shy away from having InfiniteList also re-render itself on the same event. If you're InfiniteList is the only component that would need to update on that event, then it could make sense to have the state in InfiniteList, but that seems unlikely to hold in all cases. So again, I'd lean toward B as more generic.

That being said, if you really wanted to save rewriting the event handling logic, and if you're using the same approach from the React tutorials, I would leverage this fact and just pass the Store itself as a prop. In the examples, Stores are created by inheriting from EventEmitter.prototype. If you create each of your Stores this way, you know it will have a addListener() and removeListener(), method. Additionally, you could require that each store have the methods getItemIds() and `getItem( id )'. If you wanted to explicitly enforce that the object passed into your React component has all these methods, you could use propTypes with isRequired. For example:

//YourComponent.js

var YourComponent = React.createClass({
  propTypes:{
    // assuming ListModel has been defined with the methods you need
    listModel: React.PropTypes.instanceOf( ListModel ).isRequired,
    // assuming you're using the EventEmitter, like in React example code
    modelEvent: React.PropTypes.string.isRequired
  },

  getInitialState: function(){
    // start with an empty array
    return { items: [] };
  },

  componentDidMount: function(){
    var evt_handler;

    // when the model changes, get the list again
    evt_handler = ( function(){

      this.setState( { items: this.props.listModel.getItemIds() } );
    }.bind( this ) ); // bind to maintain context

    // register for events
    this.props.listModel.addListener( this.props.modelEvent, evt_handler ); 
  },

  render: function(){
    ...do your filtering, make it pretty
  }
});

This is clearly less generic than the stateless approach of B and is really just a stylistic alternative to passing the functions themselves, but either way, I'd use propTypes with isRequired for the functions.

sethro
  • 1,997
  • 1
  • 13
  • 30
  • Excellent answer! I did end up using this method after looking at it some more, ends up being much cleaner to let the parent component handle the items. Thanks! – SleepyProgrammer Jun 05 '15 at 15:01