developit / htm

Hyperscript Tagged Markup: JSX alternative using standard tagged templates, with compiler support.
Apache License 2.0
8.67k stars 170 forks source link

Improve runtime error handling for missing close tags #166

Open dfabulich opened 4 years ago

dfabulich commented 4 years ago

Consider this sample code, which accidentally omits a closing tag.

import htm from 'https://unpkg.com/htm?module'
const h = (type, props, ...children) => ({type, props, children});
const html = htm.bind(h)

console.log(JSON.stringify(html`<h1>Hello, world!`));

In this case, the user intended to include a closing tag </h1>, and so the user's desired logged result is {"type":"h1","props":null,"children":["Hello, world!"]}

Actual: ["h1","Hello, world!"] The element name is incorrectly handled as a text node.

Expected: Throw an exception.

dy commented 4 years ago

Btw innerHTML parser does not throw errors and silently returns wrongly parsed markup. Implementing validation in parser would be size overhead. Maybe linter package?

dfabulich commented 4 years ago

I think it's worth investigating the runtime size overhead. It might not be that much.

I filed a separate bug that the transpiler also silently ignores errors; that's definitely a bad thing. The transpiler should use as many bytes as it needs to ensure correctness and report clear errors. (Plus, the transpiler itself can be a linter with babel-eslint.)

dy commented 4 years ago

investigating the runtime size overhead

Ok, some possible error cases then.

html`<a`
html`<a${'b'}>`
html`<a${'b'}`
html`<${'b'}a>`
html`<a></${''}>`
html`<a><${''}/>`
html`<a></`
html`<a ${'b'}></a>`
html`<a a${'b'}></a>`
html`<a "a${'b'}"></a>`
html`<a <!--"a${'b'}"></a>`
...

That list grows to hundreds - there are too many combinations of modes ('"quotes, comments, attrib names, attrib values, spread, fields, closing tag, tag/text, fragment). I am pretty sure there's no silver bullet, the parser is super minimalistic. Maybe transpiler indeed?

developit commented 4 years ago

It's not possible to implement runtime error checking in a way that represents a reasonable size tradeoff. This is particularly true because 100% of the classes of error that would be caught are also the classes of error that can be detected via static analysis.

Some solutions:

developit commented 3 years ago

this is probably a duplicate of #56 & #122