ryansolid / dom-expressions

A Fine-Grained Runtime for Performant DOM Rendering
MIT License
865 stars 124 forks source link

Better detection of poorly formed JSX (HTML) #105

Closed ryansolid closed 1 year ago

ryansolid commented 2 years ago

Not sure what to do yet but would be great if we could validate templates in a way that we ensure the browser won't mess with them when we instantiate them. Things like how browsers insert <tbody> if you omit it. Or the way certain elements aren't allowed certain children. This is critical because it breaks walks during render/hydration.

Right now I just count the tags at runtime which is super rudimentary and it only runs in dev mode since we don't want the additional code/overhead. In theory this can be done at compile time but I don't know of any existing packages. Maybe failing that we could just develop something ourselves?

lxsmnsyc commented 2 years ago

This principle should only apply to the templates, right?

edemaine commented 2 years ago

From https://github.com/solidjs/solid/issues/883#issuecomment-1068666676:

In case it's helpful, here is React's code for checking nesting conditions. I've definitely found these error messages helpful. Alternatively, I wonder if we could diff the serialized DOM with the input to give a helpful error message...

ryansolid commented 2 years ago

https://github.com/MananTank/validate-html-nesting

edemaine commented 2 years ago

https://github.com/MananTank/validate-html-nesting

Nice! Bundlephobia says 2.8kB minified, 1.2kB gzipped. Perhaps we could include it in dev builds, to make more helpful error messages?

ryansolid commented 2 years ago

I figure we'd run this at compile time so bundle size won't matter.

lxsmnsyc commented 2 years ago

That's one good thing, the other would be how would we validate markup that comes from components that are being appended to another markup?

For example:

<h1>
  <SomeComponent />
</h1>
ryansolid commented 2 years ago

This is ok at least from our perspective. The only thing getting force fixed by the browser are the templates I think. Maybe there are exceptions still. But like the <tbody> doesn't happen if you just append a <tr>.

lxsmnsyc commented 2 years ago

That's true, I think the compile-time works

orenelbaum commented 1 year ago

It seems like the correction is happening when setting innerHTML. Is there a reason why we can't compare the innerHTML after setting it to the input html? i.e.

export function template(html) {
  const t = document.createElement("template");
  t.innerHTML = html;
  if ("_DX_DEV_" && t.innerHTML !== html)
    throw `The browser resolved template HTML does not match JSX input:\n${t.innerHTML}\n\n${html}. Is your HTML properly formed?`;
  ...
}
ryansolid commented 1 year ago

@orenelbaum I used to do this but there were small differences like with quotes and escaped characters and stuff... so I just started counting tags. It's imperfect but I'd rather miss a few than have false positives.

orenelbaum commented 1 year ago

What matters is that the structure stays the same right? So maybe we could parse the input html and the resulting innerHTML to see if they have the same structure?

ryansolid commented 1 year ago

What Im getting at is adding more to runtime is probably just worst than looking at it at compile time.

orenelbaum commented 1 year ago

Does it matter if we run it only in dev? It seems easier to do it this way than in compile time. It would be nice in compile time to get this feedback while editing but doing it in runtime seems to me better than nothing if it's only in dev.

ryansolid commented 1 year ago

I should have closed this one a while ago when we added tag validation. Closing now.