React For loop component

Question

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.

Answer 1

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).