Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve runtime error handling for missing close tags #166

Open
dfabulich opened this issue May 4, 2020 · 5 comments
Open

Improve runtime error handling for missing close tags #166

dfabulich opened this issue May 4, 2020 · 5 comments
Labels
debugging duplicate This issue or pull request already exists enhancement New feature or request proposal

Comments

@dfabulich
Copy link

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
Copy link

dy commented May 13, 2020

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 dfabulich changed the title Improve error handling for missing close tags Improve runtime error handling for missing close tags May 13, 2020
@dfabulich
Copy link
Author

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
Copy link

dy commented May 13, 2020

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
Copy link
Owner

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:

  • compile from JSX (enables full linting & type safety)
  • write an ESLint plugin for HTM
  • Lobby for first-class Tagged Template DSL support in TypeScript and Babel

@developit
Copy link
Owner

this is probably a duplicate of #56 & #122

@developit developit added the duplicate This issue or pull request already exists label Feb 5, 2021
@developit developit added debugging enhancement New feature or request proposal labels Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugging duplicate This issue or pull request already exists enhancement New feature or request proposal
Projects
None yet
Development

No branches or pull requests

3 participants