Skip to content

React.Children.map does not return an Array or Array-like object #2872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pmeskers opened this issue Jan 16, 2015 · 8 comments
Closed

React.Children.map does not return an Array or Array-like object #2872

pmeskers opened this issue Jan 16, 2015 · 8 comments

Comments

@pmeskers
Copy link

Hey there,

I'm trying to use React.Children.map in conjunction with React.addons.cloneWithProps to create a copy of my children. However, it's surprising behavior to find that map does not return an array as a typical map function would.

The value I see in returning an array would be enabling one to perform array operations on a collection of children, such as reordering/splicing, inserting new elements, etc. Since it doesn't return an array, and the children are meant to be an opaque data structure, it's unclear to me what the best practice is for doing this type of thing.

I've created a rough jsfiddle that demonstrates this.

I assume this behavior is for optimization purposes, in which case maybe there is some other way to think about my problem.

Thanks for any feedback! Cheers.

@yiminghe
Copy link
Contributor

this confuses me too

#2856

@brigand
Copy link
Contributor

brigand commented Jan 19, 2015

In programming map f T -> T, or in english, you map a function to something of type T and get back something of type T. In JS the only built in version is an array map. This is a Children map, which maps from Children to Children. Children is an opaque data structure (no public properties or methods), so the behavior doesn't violate any promises.

React could provide a converter between Children and Array. It'd probably look something like this:

React.Children.toArray = function(children){
  var isElement = React.isValidElement(children);

  if (Array.isArray(children)){
    return children.slice();
  }
  // single child
  else if (!children || typeof children !== 'object' || isElement) {
    // avoid the warning about missing key prop
    if (isElement && !children.key) {
      return [React.cloneWithProps(children, {key: 0})];
    }
    return [children];
  }
  // {a: <div />} -> [<div key="a" />]
  else {
    return Object.keys(children).map((key) => {
      // assuming a directly set key overrides the object's key (haven't tested it)
      if (!children[key].key) {
        var props = {key: key};
        return React.cloneWithProps(children[key], props));
      }
      return children[key];
    });
  }
}

@pmeskers
Copy link
Author

Thanks @brigand for a great reply. One thing that confuses me, however, is that I'll typically invoke React.Children.map as such:

var kids = React.Children.map(this.props.children, function(child) {
    // ...
});

In this case, this.props.children is not a Children object -- however, the invocation works just fine and React.Children.map returns an instance of Children even though it was given an array argument.

I do see your point, and it's fair for the map function to not be obligated to return arrays. But it seems problematic that it accepts arrays (thus violating the formal definition of the map method you provided).

I think a larger question I have around React Elements as children is how we are meant to interact with them if we have an opaque interface. I've updated a jsfiddle here which tries to demonstrate this -- it wants to shuffle a list of children, each of which are React Elements. I would love to get some thoughts on what the best practice is for solving this problem. Your converter from Children to Array seems plausible, I'm just wondering why it hasn't been a problem for others yet -- it makes me think I'm just missing some bigger picture here :)

Cheers!

@brigand
Copy link
Contributor

brigand commented Jan 19, 2015

So... there is a way to do that, but it's complicated and involves React.Children.forEach and React.Children.count, and storing the sorted indexes in state, and handling changing the array of indexes in componentWillReceiveProps.

The better way to do it is just sort the data in the parent component. I'm glad it's so complicated to do in a child, because sorting elements doesn't make a lot of sense (you sort data).

For fun, here's an implementation.

As a side note, you shouldn't store elements in state. An element should only be used once. If I update the text of a ListItem, it wouldn't show in the UI because your component still renders the same elements.

@samhunta
Copy link

brigand, why are you using slice in your code above?

...

  if (Array.isArray(children)){
    return children.slice();
  }

...

@brigand
Copy link
Contributor

brigand commented Jan 22, 2015

@dyscrete if I just returned the array then sometimes (but not always) toArray(this.props.children) === this.props.children . That would mean that toArray couldn't promise that you're allowed to modify the returned array (if you decide you want to do that). It's mostly just a point of consistency. (If you're not aware, array.slice() is a convenient way to shallowly clone an array)

@samhunta
Copy link

@brigand Thanks for the explanation and I'm familiar with use of slice, I just didn't think it through completely before I posted that. 👍

@sophiebits
Copy link
Collaborator

map now always returns an array in 0.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants