-
Notifications
You must be signed in to change notification settings - Fork 170
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
Shouldn’t multiple root elements become a fragment? #175
Comments
This threw me off too. I was expecting I’d actually thought that this was one of the main selling-points of htm — that the explicit boundaries of the template obviated the need for explicit fragments within the string content — but it seems I misinterpreted the meaning of the “Multiple root element (fragments)” bullet point in the readme section describing improvements over JSX. |
Hiya! Multiple roots returns an Array, because that's the easiest and most efficient solution across all frameworks: const root = html`
<h1>Quiz</h1>
${entriesUi}
<hr />
<${Summary} entries=${entries} />
`;
console.log(root);
[<h1>Quiz</h1>, entriesUi, <hr />, <Summary ../>] It's not necessary to use a Fragment for this, because this is semantically an index-ordered list of children. It's best represented as an Array, because there is no useful key that could be produced for the items. React's "key" warning is only a high-level generalized piece of guidance, and it does not apply to auto-generated lists of items for which there is no viable unique key. I have a StackOverflow post that explains this more in-depth, but the gist is that this advice tends to cause people to use index keys, which is extremely harmful to diffing. At best, index keys just duplicate the behavior of unkeyed homogenous lists. In all other cases, they cause shifted and conditional items to be remounted on every render. Was the main issue here that React displayed the questionable warning, or is there a case I'm not aware of where this actually prevents usage? |
For me personally, it was that React keeps displaying a warning. Because Best fix at the moment is to either:
Being able to tersely type |
Might be worth filing a bug with React for this if it's warning in this case - key isn't useful here and that makes the warning quite misleading. |
looks like in htm you have to do this inside a nested html() template see developit/htm#175
This confused/surprised me too, and took me awhile to understand what was going on / how to avoid it. What does the readme mean by the following ?
Is it possible for HTM to automatically wrap multiple root elements in a fragment, or any harm in doing that? What are your thoughts on a terse syntax similar to JSX? <>
<p>foo</p>
<p>bar</p>
<//> |
...and/or maybe an optional config like: htm.bind( React.createElement, {
autoFragment: true,
} |
@iandunn hmm - we definitely can't do anything configuration-based in a library of this size. It's especially hard to justify given that benefit is effectively that a console warning stops being shown. Patching automatic fragment wrapping in is pretty easy: import htm from 'htm';
import React from 'react';
function html() {
const root = htm.apply(React.createElement, arguments);
return Array.isArray(root) ? React.createElement(React.Fragment, {}, root) : root;
} ... then use it like you would use Maybe the best option here would be to ship the above patch by default in |
Thanks, that sounds like a good idea to me. I'm having trouble getting it to work, though. Or, it does work in the sense that it successfully wraps the return in a Fragment, but for some reason React is still complaining about it. I diff'd the objects using that wrapper vs manually wrapping in Fragment, and don't see any difference, which is confusing. It does make me wonder, though, if it were as simple as this, then why don't things like facebook/react#8920 seems to imply that the short syntax If that's correct, then could As an aside, false-negative errors don't feel like a trivial thing to me. It may be a personality difference, but I find them very distracting, and they make it difficult to focus on what (if anything) is actually going wrong. |
This works for me:
If I don’t wrap the fragment around the roots, I’m getting this error:
(That is, HTM seems to return an Array and not a fragment.)
If my suggestion doesn’t make sense, then it may be helpful to support the
<>...</>
syntax for fragments.The text was updated successfully, but these errors were encountered: