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

Preact support: Check for empty array children #4423

Merged
merged 1 commit into from
Feb 1, 2017

Conversation

kwelch
Copy link
Contributor

@kwelch kwelch commented Feb 1, 2017

Duplicate of #4402, I added in a test.

I didn't have write access to @developit's branch so I made a fork.

Not sure if there is a different/better way to approach this test, but it seemed right to me.

@developit
Copy link
Contributor

Ah, awesome thanks for doing that @kwelch.

<Route path="/" render={() => (
<h1>{TEXT}</h1>
)}>
{[]}
Copy link
Member

Choose a reason for hiding this comment

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

The way this test is written should trigger this warning, because you're using both the <Route render> and <Route children> props. Let's just get rid of the test and put a note in the code.

Copy link
Member

Choose a reason for hiding this comment

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

Oh wait, duh. Of course the warning isn't firing because you null out children before we get that far. Ignore me :)

Question: does Preact always give you a children prop, @developit? i.e. if I render a component w/out any children like <MyComponent/> does Preact give me an empty children array?

Copy link
Contributor

@developit developit Feb 1, 2017

Choose a reason for hiding this comment

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

@mjackson yes. props.children is always a (flattened) Array in Preact, with empties removed and adjacent Strings merged. The empty children Array is always the same shared reference to an empty Array so that shallowEqual checks within shouldComponentUpdate will correctly ignore repeated empty children (for leaf nodes).

@timdorr timdorr added the feature label Feb 1, 2017
@timdorr timdorr added this to the v4.0.0 milestone Feb 1, 2017
@mjackson mjackson merged commit d822a96 into remix-run:v4 Feb 1, 2017
@mjackson
Copy link
Member

mjackson commented Feb 1, 2017

Thanks, @kwelch and @developit! 💯

@kwelch kwelch deleted the preact-support branch February 2, 2017 01:58
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants