Hyperapp: Both className and class attributes created.

[tunnckoCore]

There is classname and class attributes at the created DOM nodes.

Kinda feel that i faced that while decoupling the dom diffing and fixed it but cant remember how and where it was and where the codes are. (_just did kinda dozen rewrites of few things in a couple of folders; wrote 3-4 new packages that i am lazy to write tests and docs, haha. i'm totally :face_with_thermometer: :imp: :anger:_)

[selfup]

I think I have seen something similar here:

http://codepen.io/selfup/pen/WRgOrR

To reproduce

Make two ideas

Hit X on the latest idea (top)

The X button is still highlighted on the idea that was not clicked 🤔

---

It seems that this happens when a node replaces an old node

Exceptions

If you delete the bottom idea, the top one stays "clean"

[jorgebucaran]

@selfup It seems we're talking about two different bugs here?

One with className (safely) polluting the element's HTML and another one which is the one that can be seen in @selfup's CodePen.

Both need fixing, but I'm more interested in fixing the second one as that's a bug that has been bothering me in the TodoMVC implementation for a while.

[selfup]

We might very well be, but I am wondering if it has to do with the diffing.

If you want I can submit a new issue :smile:

Or we can just keep track of it in here

[jorgebucaran]

@selfup I think they're different, but let me investigate further, they might be related after all.

[jorgebucaran]

@selfup @tunnckoCore Could this be a bug/feature of Hyperx? This does not occur when using JSX.

[tunnckoCore]

dont think so. try raw h calls?

i'll do a pen and will see if when it appears

[jorgebucaran]

@tunnckoCore I can confirm both class and className are added when using hyperx, but definitely not when using JSX.

[selfup]

@jbucaran Both examples with issues are JSX apps.

[jorgebucaran]

@selfup Alright, can we open a new issue to track the bug you described here?

Let's keep this issue about the class/className bug.

@tunnckoCore Can you confirm this occurs when using JSX too?

[jorgebucaran]

@tunnckoCore @selfup The reason you have both classname and class is because you're using className in the JSX.

See: https://github.com/selfup/hyperapp-one/blob/master/src/views/counter.js#L10.

@selfup Consider using only class? We're not constrained by this.

[ngryman]

@jbucaran I have the same initial issue with duplicated classname and class attributes on each element.
I'm using hyperx and the class attribute (not className).
I don't have any actions, just a static model, so there is no diffing involved (afaik).

Do you have a clue on this particular bug?

[jorgebucaran]

@ngryman Yes, I just had a chance to look at this.

See this Pen.

[jorgebucaran]

Hyperx adds a classname attribute to the virtual node data object. In Hyperapp we just go ahead and set that property in the element which causes the class to be set correctly.

I don't understand why Hyperx does this, however. This is the commit that introduces the functionality in Hyperx.

• https://github.com/substack/hyperx/pull/22

[ngryman]

Ok, I get it.
Any chance this does not work for some reason: https://github.com/feross/hyperscript-attribute-to-property/blob/master/index.js#L14?

[jorgebucaran]

@ngryman Well observed, this is definitely odd and requires some investigation. 🤔

[ngryman]

@jbucaran On my local machine, only className is left so class is well deleted.
I can investigate a little on this...

[jorgebucaran]

@ngryman Wait, _it's_ working, so it's class what gets deleted.

[jorgebucaran]

Given this view:

<div class="hi">Hello</div>

JSX produces:

{
  children: ["Hello"],
  data: {
    class: "hi"
  },
  tag: "div"
}

Hyperx produces:

{
  children: ["Hello"],
  data: {
    className: "hi"
  },
  tag: "div"
}

[ngryman]

Using hyperx I have a different result.

Source

const { h } = require('hyperapp')
const hyperx = require('hyperx')
const html = hyperx(h)

const vnode = html`<div class="foo">`
console.log(vnode)

Output

{
  children: [],
  data: {
    className: "foo"
  },
  tag: "div"
}

EDIT: Didn't see you preceding comment 👻

[ngryman]

It makes sense as hyperscript-attribute-to-property proxy does replace class with className. hyperx probably expects the rendering framework to transform className into a class attribute.

[jorgebucaran]

@ngryman Hmm, that sounds right. But why?

[ngryman]

My guess is that they want to be able to put class instead of className in the markup.
It makes sense as unlike jsx, markup is a string, so there is no reserved keyword to handle.
But in the same time they also want to be react-compatible, react only supports className.

[ngryman]

Either you could patch your code to revert className to class or we could file hyperx an issue to ask them if it's possible to add some kind of opt-in for this transform.

[jorgebucaran]

🤔 That seems like out of the scope of Hyperx.

[jorgebucaran]

Either you ~could patch your code to revert className to class~ or we could file hyperx an issue to ask them if it's possible to add some kind of opt-in for this transform.

Sounds like a plan: https://github.com/substack/hyperx/issues/48.

[tunnckoCore]

hallelujah

[ngryman]

@jbucaran Another issue related to this one is that for svg elements only classname attribute is kept so it's basically impossible to style svg elements for now 😢

I have a feeling the hyperx issue may take some time to resolve, should we implement a temporary fix that just reverts className to class?

[jorgebucaran]

@ngryman You against JSX?

[jorgebucaran]

@ngryman I can get that, so then why not fork hyperx, fix the issue (send a PR along upstream too) and use your fork in the meantime.

[ngryman]

@jbucaran Good idea.
Haha I'm not against jsx but in the quest of a minimal setup with es6 native solutions ⚔️

[ngryman]

@jbucaran fyi https://github.com/substack/hyperx/pull/49

[ngryman]

PR has been merged and starting from hyperx 2.2.0 you can now disable this:

const { h, app } = require("hyperapp")
const hyperx = require("hyperx")
const html = hyperx(h, { attrToProp: false })

[jorgebucaran]

Good job @ngryman! 🍟 🍔

[jorgebucaran]

@ngryman How would a PR for hyperapp that deals with this look like?

const html = hyperx(h, { attrToProp: false })

it's kinda ugly. Maybe we can take care of it for all the Hyperx-folk using hyperapp :)

[ngryman]

I totally agree it's ugly 😢

TBH if we think about the full boilerplate, it is kinda heavy too:

import { h } from 'hyperapp'
import hyperx from 'hyperx'

const html = hyperx(h)

We should find a solution to wrap and expose this in some way.

Peer dependency

We could reference hyperx as a peer dependency and then export a helper html:

// html.js

import { h } from 'hyperapp'
import hyperx from 'hyperx'

export default hyperx(h, { attrToProp: false })

So now we can simply do:

import { html } from 'hyperapp'

New package

We could create something like hyperapp-hyperx and do the same. It's not my favorite but it preserves SoC.

@jbucaran If you have other ideas? I don't know how others deal with this.

[jorgebucaran]

We could reference hyperx as a peer dependency and then export a helper html:

It was like that when I first released the project. The problem is: I couldn't find a way to support both Hyperx and JSX in an unopinionated way.

There may be other template literal or JSX-like libraries in the future and I'd like to support them all if possible.

[jorgebucaran]

@ngryman

TBH if we think about the full boilerplate, it is kinda heavy too:

import { h } from 'hyperapp'
import hyperx from 'hyperx'
const html = hyperx(h)

There's no way about it unless we make other compromises. I tried something, but was ignored too.

See: https://github.com/substack/hyperxify/pull/8

[ngryman]

I get it, why not something like installing any template function and then get it via hyperapp?
Naming is subject to change but this is the core idea.

This would be called once to install the template function, in index.js for example:

import { app } from 'hyperapp'
import hyperx from 'hyperx'

app(/*...*/, template: hyperx)

And would be used like this in the views:

import { html } from `hyperapp'

[jorgebucaran]

@ngryman Thanks, but there's absolutely no way to go around the hyperx boilerplate, and I tried to address that in substack/hyperxify#8.

Even if we export an html module out of the box, the boilerplate will have to be written somewhere, and worse, it will be impossible to use hyperxify to remove the hyperx parser from your application bundle.

The reason for the current boilerplate is not to "punish" hyperx users, on the contrary, it's to make using hyperx with hyperapp not suck (consider the alternative, sending the entire parser down the wire, not to mention it's slow, but the bytes!).

Finally, I think the syntax you've proposed doesn't work with JSX either, so that would certainly punish JSX, which whether we like it or not, are most likely the majority.