Skip to content

Only nullify empty children array #4475

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

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

pshrmn
Copy link
Contributor

@pshrmn pshrmn commented Feb 6, 2017

#4423 added Preact support by making children null if its length is zero.

Function.length indicates how many arguments the function takes, so a function that takes no arguments has a length of 0.

<Route path='/okay' children={() => <div>What?</div>} /> // this would set children to null

This PR will make sure that children is an array before setting it to null when it is empty.

I also took the liberty of adding some basic <Route> tests for its component, render, and children props.

Copy link
Member

@timdorr timdorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise.

@@ -19,7 +19,7 @@ class Route extends React.Component {
const { component, render, match } = props // eslint-disable-line react/prop-types
let children = props.children // eslint-disable-line react/prop-types

if (children && !children.length) children = null
if (children && Array.isArray(children) && !children.length) children = null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you can just make this if (Array.isArray(children) && !children.length) children = null

@pshrmn pshrmn force-pushed the think-of-the-children branch from 8136f89 to 30eb965 Compare February 6, 2017 19:11
@mjackson mjackson merged commit 577ea93 into remix-run:v4 Feb 7, 2017
@mjackson
Copy link
Member

mjackson commented Feb 7, 2017

Thanks for another great PR, @pshrmn! 😍 those tests!

@pshrmn pshrmn deleted the think-of-the-children branch February 7, 2017 05:29
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants