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