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.

Sign up for GitHub

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

Jump to bottom

flatten children in preact.h(...)? #406

Closed
matthewmueller opened this issue Nov 16, 2016 · 5 comments
Closed

flatten children in preact.h(...)? #406

matthewmueller opened this issue Nov 16, 2016 · 5 comments

Comments

@matthewmueller
Copy link

matthewmueller commented Nov 16, 2016 •
edited
Loading

Any interest in flattening the children in preact.h(...) ?

This would allow your plugins to return arrays of children, which can be useful for things like meta tags:

<head>
   <title>Hello world!</title>
   <Metas site={props.site} />
</head>
@matthewmueller matthewmueller changed the title flatten children? flatten children in preact.h(...)? Nov 16, 2016
@developit
Copy link
Member

Hmm - h() does flatten children (here). Returning an Array from a component (pure functional or otherwise) isn't currently possible because the render path is 1:1 dom<->vdom. Some bits that limit that are resolving nested PFC's and the various parts of the codebase that expect rendering a component to produce a DOM node (whereas it would then be producing an Array of them).

I'd love to support this, I just have no idea how to do it without doubling the size of the lib. In the back of my mind I'm thinking clever use of DocumentFragments, but I haven't tried that out at all. They are a little weird post-insertion.

@thysultan
Copy link

if the component returns an array you could create a fragment VNode that has a unique signature(i.e {type: 11, props: {}, children: [...returned array]}) passing the array as its children. then use createDocumentFragment instead of createElement when you come across an element with this signature.

@leeoniya
Copy link

leeoniya commented Nov 17, 2016 •
edited
Loading

@developit @thysultan

I think the problem is not simply flattening, but actually recognizing fragment boundaries during DOM reconciliation. I explored doing this recently in domvm [1] which also has a 1:1 vdom to dom mapping. domvm supports template-level flattening but not fragment views. @trueadm also made an interesting experiment [2] for using a single comment or empty textnode placeholder to represent a fragment position but it still suffers from the following:

The main issue is fragments-within-fragments and this isnt something createDocumentFragment can solve because while they can serve as temporary containers in place of a root dom node, they get emptied once you appendChild/insertBefore them into the real dom, so become useless afterwards.

There are other issues which impact performance as well, such as moving a fragment (contiguous block of elements) into the correct position. The dom has no cheap way of moving multiple childNodes at once, so even with an optimal algo that detemines the exact new location of the fragment, you're stuck with at least a children-number of insertBefore ops.

It's possible to support fragments without too mch issue as long as you can swallow the dom reconciliation cost. But it comes with the caveat that you cannot have nested fragments without severely increasing reconciliation cost & complexity (at least in my initial attempts). I've put it on the backburner [3] for now in the interest fo getting v2 out the door, but it'd be interesting to explore/discuss some options. Take a a look through the referenced threads if curious.

[1] domvm/domvm#101 (comment)
[2] trueadm/fragment-node#1
[3] https://github.com/leeoniya/domvm/tree/defineFragment

@matthewmueller
Copy link
Author

Ahh thanks the info @leeoniya! Sounds like it's not worth the cost. Going to close this one. Thanks guys!

@developit
Copy link
Member

Wow that was an awesome explanation @leeoniya. Thanks a bunch!

@effulgentsia effulgentsia mentioned this issue Oct 17, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@leeoniya @developit @matthewmueller @thysultan