choojs / nanohtml

:dragon: HTML template strings for the Browser with support for Server Side Rendering in Node.
MIT License
687 stars 49 forks source link

Wrong attribute parsing #62

Open YerkoPalma opened 7 years ago

YerkoPalma commented 7 years ago

I'm having problems with bel. I try to use bel to render some markdown content with the help of marked library. With this code

const html = require('yo-yo')
const marked = require('marked')

const post = require('./posts/post.md')
console.log(marked(post))
document.body.appendChild(html(marked(post)))

And this markdown content

# hello world

I'm using stringify to require the .md file content. When I console log the marked(post) call, I get

<h1 id="hello-world">hello world</h1>

That's pretty valid html to me, but bel complains with this error.

Uncaught DOMException: Failed to execute 'setAttribute' on 'Element': 'hello-world"' is not a valid attribute name. at belCreateElement (http://playground-yerkopalma.c9users.io:8080/bundle.js:1447:14) at http://playground-yerkopalma.c9users.io:8080/bundle.js:1807:12 at http://playground-yerkopalma.c9users.io:8080/bundle.js:1569:45 at Object.1../posts/post.md (http://playground-yerkopalma.c9users.io:8080/bundle.js:7:27) at s (http://playground-yerkopalma.c9users.io:8080/bundle.js:1:254) at e (http://playground-yerkopalma.c9users.io:8080/bundle.js:1:425) at http://playground-yerkopalma.c9users.io:8080/bundle.js:1:443

And the problem is that when the belCreateElement function is called, it is called with wrong attributes

error

Now why is that happening? I thought about an hyperx error, but tried it with hyperscript and it render correctly. Not sure what could it be, also looked at the hyperscript-attribute-to-property library inside hyperx, but again why would it work with hyperscript, but not with bel?

Any help would be appreciated.

roobie commented 7 years ago

I've experienced the same issue. Funny thing is:

This does not work, as @YerkoPalma states above:

html(htmlAsText)

However, this works:

eval('html`' + htmlAsText + '`');

Which leads me to the suspicion that there is some conditional somewhere in the library that makes the parsing go differently depending on whether it is used as a template string or as an ordinary function.

(Tested in Chromium Version 55.0.2883.75 built on Debian stretch/sid, running on Debian 9.0 (64-bit))

yoshuawuyts commented 7 years ago

I don't completely follow why you're trying to call it through bel this way; it'd break when using yo-yoify. If you're looking for an example of how to use bel + marked I did some work for the choo handbook here https://github.com/yoshuawuyts/choo-handbook/blob/master/index.js. Hope this helps (:

On Tue, Jan 31, 2017 at 2:17 PM Björn Roberg notifications@github.com wrote:

I've experienced the same issue. Funny thing is:

This does not work, as @YerkoPalma https://github.com/YerkoPalma states above:

html(htmlAsText)

However, this works:

eval('html' + htmlAsText + '');

Which leads me to the suspicion that there is some conditional somewhere in the library that makes the parsing go differently depending on whether it is used as a template string or as an ordinary function.

(Tested in Chromium Version 55.0.2883.75 built on Debian stretch/sid, running on Debian 9.0 (64-bit))

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/shama/bel/issues/62#issuecomment-276511120, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlescufBRr8mqLykxlhFdIOI7iqey4ks5rX7LxgaJpZM4Li2eB .

roobie commented 7 years ago

Personally, I used 'choo/html' but the Error is thrown in bel, which is why I chimed in here, but I may be a bit off =)

yoshuawuyts commented 7 years ago

No that's cool, but what I'm saying is that calling bel() as a function instead of a tagged template literal is something you should probably never do - instead there are probably better ways to achieve what you're trying to achieve. (e.g. DOMNode.setInnerHTML and the like)

On Tue, Jan 31, 2017 at 3:06 PM Björn Roberg notifications@github.com wrote:

Personally, I used 'choo/html' but the Error is thrown in bel, which is why I chimed in here, but I may be a bit off =)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/shama/bel/issues/62#issuecomment-276522701, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlekSuZE6X1CUPb60_d8pVvirdPDzqks5rX75mgaJpZM4Li2eB .

blahah commented 7 years ago

@yoshuawuyts I use html(foo) quite a lot in choo. It's useful whenever you want to use some externally supplied HTML. Can you elaborate on why it's a bad idea? Apart from this issue of course, which seems like a bug.

yoshuawuyts commented 7 years ago

Oh cool, didn't know folks did that - guess it wasn't an intentional consequence but ueh yeah that's actually pretty neat.

On Sun, Feb 26, 2017 at 2:21 PM Richard Smith-Unna notifications@github.com wrote:

@yoshuawuyts https://github.com/yoshuawuyts I use html(foo) quite a lot in choo. It's useful whenever you want to use some externally supplied HTML. Can you elaborate on why it's a bad idea? Apart from this issue of course, which seems like a bug.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/shama/bel/issues/62#issuecomment-282555523, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWletHXXIuQxxR_M6MJxXYiUh4XScdEks5rgXxFgaJpZM4Li2eB .

roobie commented 7 years ago

@blahah you've never had the DOMException thrown from bel when doing that?

blahah commented 7 years ago

@roobie I have, only with certain elements. It always happens with <img src="..." />, but quite often I don't get the error.

I was suggesting that the usage pattern of passing HTML to html() is useful, and questioning why it would be a bad idea. I'd love to see this solved.

YerkoPalma commented 7 years ago

I guess that the problem might be related with yo-yoify. As it is a browserify transform, it can make a difference if it is called like

bel(str)

or like

bel`${str}`

Because of the parsing made by yo-yoify. Also, it is only my thoughts as I havent checked the code yet.