search

WebReflection / uhtml

A micro HTML/SVG render
MIT License
916 stars 37 forks source link

Bug? Rendering as dom element broken for 4.4.1, not 4.3.5 #106

Closed greghuc closed 9 months ago

greghuc commented 9 months ago

Quick bug report, discovered on upgrading project from uhtml 4.3.5 to 4.4.1:

var htmlAsNode = require('uhtml/node').html;
var cssText = "p { font-weight: bold; }";
var result = htmlAsNode`<style>${cssText}</style>`;
console.log(result);

uhtml 4.3.5
=> <style>p { font-weight: bold; }</style>
=> is working style element

uhtml 4.4.1
=> <style><!--isµ0--></style>
=> is not working style element
WebReflection commented 9 months ago

Uhm … the node export might need a render indeed … curious to know why you use it but my bad, I haven’t tested that but it’s easy to solve. Apologies for any inconvenience

WebReflection commented 9 months ago

P.S. even more curious to know if you ever used fragments as node as the persistent fragment is the only option I have there and I’m worried in node export might not always survive.

greghuc commented 9 months ago

"the node export might need a render indeed … curious to know why you use it" => I'm using uhtml for different projects. Sometimes I need to create a one-off dom tree, so I use "'uhtml/node".html. In the example above, I'm creating a one-off style element from the cssText variable, as I've calculated a sha256 hash from cssText, for use as a style-src hash in a csp policy.

"curious to know if you ever used fragments as node as the persistent fragment" => I'm not familiar with fragments. If you have the time, could you please explain, or provide an example (maybe for the bug report above?)

Thanks

greghuc commented 9 months ago

Basically, most of the time, I use uhtml as the render part of my own tiny model-view-controller framework. I use uhtml to render and diff-update a view when the underlying model changes. However, I use node export for one-off situations where I want a dom tree, and no clever diff-updates will every be done to the generated tree. Ideally uhtml would cover both use-cases, which it does right now.

WebReflection commented 9 months ago

Fixed. There is a bump in size but at least it's 100% compatible with regular templates. All you skip in uhtml/node is the cache so that everything else stays the same ... full unroll capability with arrays, fragments, and so on.

Fragments are persistent in here but you need to use the render or always remember to use ref.valueOf() before appending these.

// single span
const span = html`<span />`;
document.body.replaceChildren(span);
// <body><span></span></body>

// persistent fragment with multiple spans
const spans = html`<span /><span /><span />`;
document.body.replaceChildren(spans.valueOf());
// <body><span></span><span></span><span></span></body>

Once you use valueOf() which is a no-op for any other kind of node so it's always safe to invoke, you can reference that fragment as it is and re-append later on anywhere you like in case it gets lost/removed.

greghuc commented 9 months ago

@WebReflection thanks for the quick fix. The buggy code is now working. Based on what you said about .valueOf, I now have:

var htmlNode = require('uhtml/node').html;

function htmlAsNode(...args) {
  var result = htmlNode(...args);
  return result.valueOf();
}
WebReflection commented 9 months ago

for one off operations where you never want to reuse that node you don't need to do anything different ... but in case you ever want to reuse that node you can just:

const htmlAsReusableNode = (...args) => htmlNode(...args).valueOf();
greghuc commented 9 months ago

What do you mean by "reusable node"?

I was assuming that the returned value of htmlNode(...args) is just a vanilla dom-element tree (say result1). And if I called htmlNode(...args) again with the same values, I would get a brand-new vanilla dom-element tree (say result2). And:

WebReflection commented 9 months ago

by reusable I mean you can re-append it in the future.

// this always works as unique node
const div = html`<div />`;

// this works only the first time if no valueOf is used
const fragment = html`a<button />c`;

document.body.append(div, fragment);

setTimeout(() => {
  // this fails, unless you use valueOf
  div.append(fragment);
});

If that fragment would have been created like this instead:

const fragment = html`a<button />c`.valueOf();

// you can now re-move or re-append that reference
// and all its nodes will move together so previous case
// just works

Two calls to the same template always produce two different nodes unless you use the uhtml/keyed and both the reference and the key are the same ... none of your business here though, it's all good, just watch out fragments, if you ever need those.

greghuc commented 9 months ago

Thanks!

WebReflection commented 9 months ago

@greghuc you know what? this was dumb (from my side) ... latest just does that for you so you don't need to worry about anything.

greghuc commented 9 months ago

@WebReflection cool. So from latest code, it looks like .valueOf doesn't need called on result of html function any more, as it's called internally as part of tag function?: https://github.com/WebReflection/uhtml/blob/main/esm/node.js#L8

greghuc commented 9 months ago

So my original code should work without any changes..

WebReflection commented 9 months ago

if you are on latest, yes!