dom-expressions icon indicating copy to clipboard operation
dom-expressions copied to clipboard

Better detection of poorly formed JSX (HTML)

Open ryansolid opened this issue 2 years ago • 8 comments

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?

ryansolid avatar Jan 23 '22 03:01 ryansolid

This principle should only apply to the templates, right?

lxsmnsyc avatar Jan 27 '22 08:01 lxsmnsyc

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...

edemaine avatar Apr 04 '22 16:04 edemaine

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

ryansolid avatar May 18 '22 05:05 ryansolid

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?

edemaine avatar May 18 '22 14:05 edemaine

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

ryansolid avatar May 19 '22 07:05 ryansolid

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>

lxsmnsyc avatar May 19 '22 10:05 lxsmnsyc

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>.

ryansolid avatar May 19 '22 20:05 ryansolid

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

lxsmnsyc avatar May 20 '22 01:05 lxsmnsyc

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?`;
  ...
}

orenelbaum avatar Dec 19 '22 10:12 orenelbaum

@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.

ryansolid avatar Dec 20 '22 18:12 ryansolid

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?

orenelbaum avatar Dec 23 '22 18:12 orenelbaum

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

ryansolid avatar Dec 23 '22 18:12 ryansolid

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.

orenelbaum avatar Dec 23 '22 19:12 orenelbaum

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

ryansolid avatar Jul 05 '23 19:07 ryansolid