luwes / sinuous

🧬 Light, fast, reactive UI library
https://sinuous.netlify.app
1.05k stars 34 forks source link

Fragile HTML parser -> create Debug bundle #75

Open mindplay-dk opened 4 years ago

mindplay-dk commented 4 years ago

In this example, I didn't explicitly close a couple of <input> elements:

https://codesandbox.io/s/vigilant-goodall-hqh8u

That is, <input> and other HTML void elements seem to require a trailing slash, e.g. <input /> - but there is no error message when void elements aren't explicitly terminated, and you get some strange results in the DOM.

Is the parser XHTML by design?

Handling HTML 5 void elements shouldn't require much work, and shouldn't add much to the footprint, I think - I wrote a toy HTML 5 parser myself in 0.5 KB, and the magic is less than 100 bytes.

luwes commented 4 years ago

Good question @mindplay-dk. To be honest I've stumbled upon this issue myself where I forgot the closing slash. Sinuous's HTML parser is 95% https://github.com/developit/htm. I'm not fully sure why they didn't add self closing tags; performance or size.

mindplay-dk commented 4 years ago

Hmm, yeah...

htm is JSX-like syntax in plain JavaScript

So I guess the name is a little misleading - it's not trying to be a substitute for HTML, but for JSX, so maybe that's why the parsing rules are XML-like, which JSX is too...

luwes commented 4 years ago

closing this one as I think it's a fairly small inconvenience in devex.

I personally rather add a closing slash that gets compiled away than to add an extra 100 bytes to my bundle (OCD 😄)

mindplay-dk commented 4 years ago

closing this one as I think it's a fairly small inconvenience in devex.

Honestly, it would be, if it would at least error.

Is there some way to make it throw for mismatching opening/closing tags?

I haven't looked at the source, but you must have a stack of some sort that keeps track of open/closed elements? Probably should check if a closing element matches the one on the stack and throw if it doesn't. Maybe check the stack length at the end of an operation, and throw if it's non-empty.

Just getting broken HTML is not good DX imo - besides being difficult to debug, this can can lead to silent bugs that end up in production.

These are syntax errors, which really shouldn't just be ignored. Silent bugs are the worst.

I personally rather add a closing slash that gets compiled away than to add an extra 100 bytes to my bundle

I'm down with that - and as said, if the intent is to work like JSX, that's based on XML and not HTML, so you'd expect to have to close your tags.

But JSX parsers will error if you forget to self-close (or close) any tags.

luwes commented 4 years ago

good points! we could possibly add a debug bundle with more error messaging that would make this visible in the the development environment.

Freak613 commented 3 years ago

Definitely shouldn't be done on runtime. Same as JSX parsers do not work in runtime. ESLint seems good place to do such checks. There is one plugin for lit-html, probably worth to try: https://github.com/43081j/eslint-plugin-lit