Hyperhtml: Space around = sign in attribute throws error

[adotg]

This works fine

function show(render, state) {
    render`
      <div id="d1" class="panel" style=${state}>
        some other text
      </div>
    `;
  }

 show(hyperHTML.bind(document.body), {background: 'lightblue'});

while

function show(render, state) {
    render`
      <div id ="d1" class = "panel" style=${state}>
        some other text
      </div>
    `;
  }

  show(hyperHTML.bind(document.body), {background: 'lightblue'});

throws an error saying Uncaught TypeError: this[(i - 1)] is not a function.

Notice the space around equal sign in the latter code snippet.

Spaces (\s) before equal sign are valid html character.
Spaces might have a usecase

• Better readability
• Conventions
• Dynamic creation of the template

I spent almost 2hrs on this thinking something in my app is causing the issue :|

Pull request 223

[WebReflection]

I spent almost 2hrs on this thinking something in my app is causing the issue :|

sorry to hear that, by honestly, none of your points is valid, for my eyes:

• worse, also inconsistent, readability (one =" and one = " ???)
• nobody in my experience ever used such convention, and I've been around for 18+ years in the field
• dynamic creation of template should be done properly, specially if you create dynamically, on the backend, JavaScript template literals, which is IMO some sort of a nonsense to me, but hey, dynamic is dynamic ...

I need to do performance test 'cause the RegExp is already pretty complex, but overall I am not sure it makes sense to support edge cases like these, it looks like your automatic JS generator should produce cleaner output?

[adotg]

worse, also inconsistent, readability (one =" and one = " ???)

attr =val is inconsistent for sure but I have seen people using attr = val and IMHO it's okay, resembles to the mental model of property assignment. But if it's a bad practice / something the standard disdain then its completely fine.

nobody in my experience ever used such convention, and I've been around for 18+ years in the field

    <div
        id = "id",
        class = "class"
        onclick = "drag"
                contenteditable = true
        style = "prop: val; propagain: valagain;"
    >
        content
    </div>

I have seen this kind of code before. Where the list of attrs on a tag is pretty massive. Could be a wrong approach, people misuse it because raw html does not bring any space constrain with its behaviour and there is no guidlines. (That does not mean the lib shouldn't. I was just stating what I dealt with)

I need to do performance test 'cause the RegExp is already pretty complex, but overall I am not sure it makes sense to support edge cases like these

Sure. I think its fair if a lib does not support all the peculiarity of hosting env (in this case html) and concentrate on one thing. But in that case IMO a note on the doc would be helpful.

it looks like your automatic JS generator should produce cleaner output?

I dont have auto js generators or something. I placed the space by mistake. :-/

[WebReflection]

like I've said, I'd like to bench the RegExp first and then make a decision. If standards say that's OK then let it be, it's just something I've never seen in the wild.

I placed the space by mistake.

indeed that's the only use case I had in mind :smiley:

[sourcegr]

And it is a common use case, unfortunately. I myself had the same issue, and struggled for some hours until I nailed it! Never bothered to file a bug, though... I never thought of it as something that the library should be aware about.

But, for what it's worth it would be a good thing to add, mostly because if a space is inerted by accident, there is no error message and no clue at all as to what has happened and why the whole thing broke :(

[WebReflection]

unfortunately, that change breaks few things, and there is also utils.js to deal with.

I think I'll try to find some time to fix this but right now it's very low priority.

[sourcegr]

bit a coworker again...
About 2 hours wasted, until he asked a review from me.
:(

Now, he knows...

[WebReflection]

I'm afraid I have no time for this so PRs that don't break everything else are welcome.

This is super low priority, not sure where this writing attributes with spaces practice comes from though, it'd be awesome if people would just write like every single example of the W3C site, or Mozilla MDN, is written, for what I remember.

[sourcegr]

This is super low priority, not sure where this writing attributes with spaces practice comes from though

It is not a practice. Accidents do happen. And yes, we all understand and respect taht this is extra low priority :)

it'd be awesome if people would just write like every single single example of the W3C site, or Mozilla MDN

Actually - if you think about it - standards do not forbid you to use spaces around = for attributes. So, we either fix it or, since we break that rule and demand from our users to ditch the spaces, we should at least mention it to the docs!

Anyway, it just happened to me again, and I though I should mention it again (kind of reminder)

[jiayihu]

IMO the best way to solve the issue at the moment is providing an error to the dev. Uncaught TypeError: this[(i - 1)] is not a function happens also if you try partial attributes, which are documented, but someone might have missed the doc section or not remember it immediately.

I mean something like the following:

function update() {
  const length = arguments.length;
  for (let i = 1; i < length; i++) {
    if (!this[i - 1]) throw new Error(`attribute=value is the correct usage, partials attributes and whitespace are not supported.`)
    this[i - 1](arguments[i]);
  }
}

[WebReflection]

@sourcegr can you please verify that latest version v2.19.0 fixes this?

[WebReflection]

@jiayihu can you also please try latest? there is now an error when the length of the updates is different from the expected one so that the error should be at least more meaningful, thanks.

[WebReflection]

@adotg I have tested in this Code Pen your initial example and I can confirm the domtagger is compatible with those now.