Skip to content
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

Rename children data from config to props for jsx components #137

Closed
mairatma opened this issue Aug 1, 2016 · 7 comments
Closed

Rename children data from config to props for jsx components #137

mairatma opened this issue Aug 1, 2016 · 7 comments
Assignees

Comments

@mairatma
Copy link
Contributor

mairatma commented Aug 1, 2016

Since JSX components have a state/props separation, it makes more sense to call the data from each child in the this.props.children array props, instead of config.

We'll probably need to keep both config and props for now, so that this won't be a breaking change (then when we launch a version 3.x someday we can keep just props for jsx).

@mairatma mairatma self-assigned this Aug 1, 2016
@eduardolundgren
Copy link
Contributor

We decided to remove config on 2.x.x, why is it still there?

@mairatma
Copy link
Contributor Author

mairatma commented Aug 1, 2016

This is not the component variable config we're talking about. It was indeed removed.

This is the config information for children components, that the parent receives so that it can access what the children received if it wants to. Soy doesn't have this feature, as it can't handle javascript objects inside soy templates, but in jsx it's a pretty common use case.

For example, let's say we have some code like this:

<Parent>
  <Child foo="foo" bar="bar" />
  <Child foo="foo2" bar="bar2" />
</Parent>

Parent can then render the children it receives by using its this.props.children property, like this:

class Parent extends JSXComponent {
  render() {
    return <div>{this.props.children}</div>;
  }
}

Instead of just rendering them directly, the parent can choose to render just a few of its children components, as well as inspect the data that they've received, For example:

class Parent extends JSXComponent {
  render() {
    <div>
      Number of children: {this.props.children.length}
      Prop "foo" from first child: {this.props.children[0].config.foo}
      Prop "bar" from second child: {this.props.children[1].config.bar}
    </div>
  }
}

The problem I'm mentioning here, and which was reported to me by @mthadley, is that even though foo and bar are now called props in jsx components, you still need to access your children's config variable to see them, which is weird. That should be named props instead, so that you could do, for example: this.props.children[0].props.foo.

@eduardolundgren
Copy link
Contributor

In this case it won't require a 3.x.x cut, it's a bug fix.

@eduardolundgren
Copy link
Contributor

If we decide to follow what's proposed here #138 we can keep both as of 2.x.x+.

@eduardolundgren
Copy link
Contributor

Let's test here the backward compatibility strategy proposed #138 (comment)?

@mairatma
Copy link
Contributor Author

mairatma commented Aug 1, 2016

Will do

@mairatma
Copy link
Contributor Author

mairatma commented Aug 3, 2016

This is now available at version 2.2.1.

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

No branches or pull requests

2 participants