3

I'm writing a small component that decorates some children. Given the following sample:

const Child = props => <div>{props.name}</div>;
const Parent = props => {
  let wrappedChildren = React.Children.map(props.children, c => {
     return (<Decorator key={c.key}>
       {c}
     </Decorator>);
  });

  return (<div>
    {wrappedChildren}
  </div>);
}
const Consumer = props => {
     let children = [0, 1, 2].map(num => {
       return <Child key={num} name={num} />;
     });
    return <Parent>{children}</Parent>;
});

In this code, I'm wanting to take each child and decorate it with some wrapping container or some behaviour. Forgetting for the moment that there may only be one child, I need to give each instance a key.

Currently I'm assuming that each child does have a key which isn't fantastic, lifting it off the child and applying it to the Decorator directly.

Is this the "correct" way of doing this? Is there a better way?

Ray Booysen
  • 28,894
  • 13
  • 84
  • 111
  • what if you pre-generated an array of UUID's and then used the `context` api in react to reach into this list as needed? – maxwell Jun 21 '17 at 14:59
  • I think that your approach is fine. As a user of a Grid component, if you create multiple children within array, then most likely you are going to set keys for them. If these children are added directly (as you have said it might be buttons, divs, etc), then they probably won't have a key. Knowing that some children might or might not have a key, decorator should check for it's presence, and add a key or proceed without a key respectively. This way decorator won't bring anything new to the children re-render process and a Grid user will have full control of it. – Michael Radionov Jun 21 '17 at 21:07

3 Answers3

3

That would work with the current version of React, 15.6.1 (and probably with prior versions as well). However, there is a slightly better way to achieve your goal with a small tweak, which would be delegating the lifting on a prop, rather than using directly the key.

The reason is that the concept of a key is something that is controlled by React internals before your component gets created, so it falls into the implementation details category, with the consequence that future versions of React could break your code without you even noticing it.

Instead, you can use a prop on your Child component, for instance id, based on your assumption that each child does have some sort of unique value. Your code would result in:

const Child = props => <div>{props.name}</div>;

const Parent = props => {
  return React.Children.map(props.children, c => {
     return (<Decorator key={c.props.id}>
       {c}
     </Decorator>);
  });

  return (<div>
    {wrappedChildren}
  </div>);
}

const Consumer = props => {
     let children = [0, 1, 2].map(num => {
       return <Child id={num} name={num} />;
     });
    return <Parent>{children}</Parent>;
});

If you have a more complex structure and want to avoid passing props individually, the parent component should held the responsibility of generating unique ids for the children. You can follow the ideas explained in this answer https://stackoverflow.com/a/29466744/4642844.

Those are the steps I'd follow:

  • Implement your own utility function or use an existing browser library to generate unique ids.
  • Implement componentWillMount in your Parent component to create an array of unique ids. You can use an internal member class variable instead of local state. It would be something like:

    componentWillMount() {
      this.uuids = React.Children.map(() => uuid());  
    }
    
  • Inside your Parent render, assign each key to the Decorator component in the following way.

     render() {
       return React.Children.map(props.children, (c, i) => {
         return (<Decorator key={this.uuids[i]}>
           {c}
         </Decorator>);
       });
     }
    

Some useful links:

http://mxstbr.blog/2017/02/react-children-deepdive/

https://medium.com/@dan_abramov/react-components-elements-and-instances-90800811f8ca

rgommezz
  • 13,536
  • 2
  • 18
  • 23
  • This does work, but maybe my example was a bit small. Imagine a Grid component that takes any type of child, it may be a div it may be a button, or some custom component. I'd have to add an id prop to every prop type – Ray Booysen Jun 21 '17 at 08:08
  • BTW, I completely agree with Key being an internal impl detail. :) – Ray Booysen Jun 21 '17 at 08:39
  • I added a proposal for skipping passing props explicitly in the child. – rgommezz Jun 21 '17 at 14:38
3

I think your approach is fine. And you do need the key on the top level. Use the child's key, if it is there. If not, fall back to the index, as React recommends:

When you don't have stable IDs for rendered items, you may use the item index as a key as a last resort.

Be advised though:

We don't recommend using indexes for keys if the items can reorder, as that would be slow.

Source: React Docs about keys

const Child = props => <div>{props.name}</div>;
const Parent = props => {
  let wrappedChildren = React.Children.map(props.children, (c, i) => {
    const key = c.key ? `key-${c.key}` : `index-${i}`
    return (
      <Decorator key={key}>
        {c}
      </Decorator>
    );
  });

  return (
    <div>
      {wrappedChildren}
    </div>
  );
};
const Consumer = () => {
  let children = [ 0, 1, 2 ].map(num => {
    return <Child key={num} name={num} />;
  });
  return <Parent>{children}</Parent>;
};
MoeSattler
  • 6,684
  • 6
  • 24
  • 44
-1

Javascript Array.map(fn) passes actually 3 arguments to fn second being an index. So, what you need to do is: let wrappedChildren = props.children.map((c, i) => <Decorator key={i}>{c}</Decorator>);

  • I definitely don't want to use the index as the key. If the child has a key, would prefer to do that. In this case, using the index means issues when the array reorders. I checked the docs, you're not using Array.map(), this is the map function on the children data structure – Ray Booysen Jun 20 '17 at 15:46
  • How about using `React.Children.toArray` and than `map`? What problems do you see if your array is rearranged? – Misha Tavkhelidze Jun 20 '17 at 17:24
  • 1
    As per the docs: https://facebook.github.io/react/docs/lists-and-keys.html We don't recommend using indexes for keys if the items can reorder, as that would be slow. You may read an in-depth explanation about why keys are necessary if you're interested. – Ray Booysen Jun 20 '17 at 21:00