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

add props.children to StatelessComponent #10641

Closed

Conversation

donaldpipowitch
Copy link
Contributor

@donaldpipowitch donaldpipowitch commented Aug 16, 2016

case 2. Improvement to existing type definition.

Handles props.children in StatelessComponent as in Component. Treats <Foo children={Bar} /> in the same way as <Foo><Bar /></Foo> with const Foo: StatelessComponent = (props) => <div>{props.children}</div>.

Handles `props.children` in `StatelessComponent` as in `Component`. Treats `<Foo children={Bar} />` in the same way as `<Foo><Bar /></Foo>`.
@paulvanbrenk
Copy link
Contributor

Reading the comment on Component this seems like a workaround.

@RyanCavanaugh you can an opinion?

@donaldpipowitch
Copy link
Contributor Author

Any update on this? :) // cc @RyanCavanaugh

@RyanCavanaugh
Copy link
Member

I think this is an OK change for now. We'll probably be handling the question of children in TS 2.1 but this will be a good workaround in the meantime.

@donaldpipowitch
Copy link
Contributor Author

Just as a reference for others: you're speaking about microsoft/TypeScript#8588, right?

@RyanCavanaugh
Copy link
Member

That's right

@zhengbli zhengbli added Revision needed This PR needs code changes before it can be merged. and removed Awaiting reviewer feedback labels Oct 18, 2016
@zhengbli
Copy link
Contributor

@donaldpipowitch can you update the PR to resolve the merge conflict? Then it looks like it is ready to merge. Thanks!

@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented Oct 19, 2016

I'd like to do that after #11935 has been merged. It touches the same line, but is way more important. I don't want to introduce more barriers there.

@vvakame vvakame added the @types label Oct 25, 2016
@minestarks
Copy link
Contributor

@donaldpipowitch Looks like #11935 has been merged. Do you plan to resolve the merge conflict? As this PR is getting stale, I plan to close it out soon if no changes. Thank you.

@donaldpipowitch
Copy link
Contributor Author

@RyanCavanaug You said "I think this is an OK change for now. We'll probably be handling the question of children in TS 2.1 but this will be a good workaround in the meantime."

2.1 is around the corner, was this solved or is this workaround still useful?

@donaldpipowitch
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revision needed This PR needs code changes before it can be merged. @types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants