# React For loop component

This is my React component for looping a component i.e. I don't want to use MAP or FOREACH in React Component's HTML.

import React from 'react';

/**
* @param {items} from props
* @description -> Works with array of items and render only one children object inside it, ith data object will be available in children component props.
*/

const RFor = (props) => {
const { items } = props || [];
return items.map((data, index) => <div key={index}>{React.cloneElement(props.children, data)}</div>)
}

export default RFor;


And this is how I use to iterate my component.

<RFor items={TEMP_DATA_ARRAY}>
<Item />
</RFor>


What this will do is generate <Item /> till TEMP_DATA_ARRAY and each object is going to be available with data property inside <Item />. I want to optimize it and this is the best approach I can come up with.

This approach is not bad but I'd highlight a few issues:

• You wrap each element with a <div>, it's unnecessary and for a very long list you create many useless elements. This also prevents <RFor> to be conveniently used (for example) to generate rows in a table.
• You're assigning data to the properties of each item. While this might be convenient it's also fragile: what if an object as a property named key, or children, ref or...anything else already used? I'd assign data to a single property.
• const { items } = props || [] does not do what you, probably, think it's doing. [] won't be the default if items is falsy but the default if props is falsy. The correct syntax, if the value is undefined is const { items = [] } = props;. If you want to handle also null then you can't destructure: const items = props.items || [];.
• To use index as key is, more often than not, a terrible idea if the list isn't fixed (for example if you can remove/add elements anywhere in the list). Leave this responsibility to the caller because it's the only one with enough knowledge to pick a sensible unique ID (which in some very specific cases can even be the index).
• You're passing props.children directly to React.cloneElement however it expects a single element and children can be an array. Enforce this rule using React.only().

To put things together (leaving out the key for the moment):

const RFor = props => {
const items = props.items || [];
return items.map(data => React.cloneElement(React.only(props.children), { data });
};


All these said you may start wondering if you even need RFor, let's compare with few alternatives:

<RFor items={TEMP_DATA_ARRAY}>
<Item />
<RFor>


Vs

{TEMP_DATA_ARRAY.map(data => <Item key={data.id} {...data} />)}


Or:

{TEMP_DATA_ARRAY.map(data => <Item key={data.id} data={data} />)}


I don't see any benefit (note that you may, instead of children use a property like:

<RFor items={TEMP_DATA_ARRAY} component={Item} />


Also note that to solve the key problem you might use a render function:

<RFor items={TEMP_DATA_ARRAY}>
{data => <Item key={data.key} data={data} />}
<RFor>


RFor IMHO makes sense only if it adds any other value over map(), it might be filtering or it might be part of an abstraction to build a presentation component with templates (with properties like controlTemplate, headerTemplate and itemTemplate as you often see in WPF).

• Hi! Thanks for your answer. I really appreciate it. Well, RFor is just made for abstraction! And you can clearly see the difference between the code you wrote with RFor vs Map. RFor is much readable. Anyway thanks. Feb 22 '19 at 12:23
• Hmmmm well, if there isn't any other added value then I don't feel it's any more readable than a plain simple map() (especially because you REALLY have to address the key issue then you probably end-up with a render function) but that just my personal POV (even if I appreciate the dom-repeat thing in Polymer). Feb 22 '19 at 12:25