Hyperx does not handle void elements + unquoted attributes correctly

[josephg]

In the HTML spec, some elements are void and don't have closing tags:

Void elements area, base, br, col, embed, hr, img, input, keygen, link, menuitem, meta, param, source, track, wbr

But in hyperx, when combined with unquoted attributes they cause weird problems:

const hx = require('hyperx')(console.log)
hx`<span><input type=checkbox>some content</span>`

This should generate a span with 2 children (an input element and 'some content'). Instead, it tries to place the content inside the input element.

input { type: 'checkbox' } [ 'some content' ]
span {} [ undefined  /*input*/ ]

If I put quotes around the attribute (<input type="checkbox">) or put a useless space before the close of the tag > then it works:

input { type: 'checkbox' } undefined
span {} [ undefined /*input*/, 'some content' ]

One workaround is to always add quotes, but thats not possible when the last attribute is an event handler, eg this breaks:

hx`<span><input onchange=${myhandler}>some content</span>`

And the event handler doesn't work if its quoted.

[yann300]

seems that change breaks something but I don't really know what neither how... https://travis-ci.org/ethereum/browser-solidity/builds/282778829?utm_source=github_status&utm_medium=notification I have this error: multiple root elements must be wrapped in an enclosing tag since today, I guess it's since the version bump. note: i have attributes with double quotes in the code.. like: <div class="${css2.overlay} ${css2.text}"></div> . perhaps it's because of that ...

[yoshuawuyts]

@yann300 uh oh; that's no good. Could you perhaps PR a test for it? We should fix this ASAP

[josephg]

Uuh what? It looks like travis is trying to test hyperx using ruby configuration: https://travis-ci.org/choojs/hyperx/jobs/282733862/config

But also @yann300 I'm not going to make a PR against your project, but I just spent some time looking into it. The problem is this line:

          <input type="text" class=${css.filter} onkeyup=${filter}></div>

The DOM here is supposed to be balanced. You're closing a supposedly void element (input) with </div>. In your case, the two bugs (your html and hyperx's previously buggy void element handling) cancelled each other out. But now one of those bugs is fixed, and it is (correctly) refusing to compile. I didn't think of that, but I guess fixing this issue is revealing some other bugs in users' code if they're depending on this bug for their malformed HTML to compile.

This brings up another issue that I really wish hyper addressed - I've seen things like this in the wild way too often with choo. The ergonomics of fixing issues like this is terrible:

• Often you don't know there's a problem because hyperx will match any opening tag with any closing tag, so often malformed html just sort of silently works
• When there is a parse error there's not even a line number in the stack trace. I traced this one down by binary searching using /* */.

At least in my previous job, it'd have been really convenient for hyperx to have a strict mode.

[yoshuawuyts]

But now one of those bugs is fixed, and it is (correctly) refusing to compile. I didn't think of that, but I guess fixing this issue is revealing some other bugs in users' code if they're depending on this bug for their malformed HTML to compile.

Good catch. I kinda wish we had better ergonomics pointing out errors like these – for people whose code now suddenly broke, I can imagine it would feel rather frustrating that their code no longer works, even if it was never supposed to run in the first place. @josephg if you have any ideas how to improve this, help would be very welcome!

@yann300 sorry your code broke; I hope it'll not be too much of a hassle to fix the newly pointed out errors and get things running again! ☺️

[josephg]

@josephg if you have any ideas how to improve this, help would be very welcome!

No - sorry! We could wrap parsing in a try/catch which would re-parse using the old code if parsing failed with the new option. But thats awful. Sorry :/

I hope not too many people are affected. i suspect most people would have used <input type=text /> and that still works fine. (Does <input type=text></input> work? If not we could fix that)