custom element attributes are returned as children rather than properties

[hesxenon]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

8

Link to runnable example

https://jsbin.com/mivorid/2/edit?html,console

Steps to reproduce

simply calling h("my-custom-element", {type: "date", value: ""}, []) yields unexpected results.

Expected behavior

It should yield

{
    "type": "element",
    "tagName": "my-custom-element",
    "properties": {
        "type": "date",
        "value": ""
    },
    "children": []
}

Actual behavior

It does yield

{
    "type": "element",
    "tagName": "my-custom-element",
    "properties": {},
    "children": [
        {
            "type": "date",
            "value": ""
        }
    ]
}

Affected runtime and version

[email protected]/[email protected]

Affected package manager and version

No response

Affected OS and version

No response

Build and bundle tools

No response

[hesxenon]

well, it seems like a rather big issue... this function here seems to be used to differentiate between estree nodes and properties and since both estree nodes and properties may have a type and value property the function incorrectly returns false. An exception for <input> is in place, but that won't help with custom elements.

Is branding a possibility?

[wooorm]

Exception for dash in tag name might be viable?

Can custom elements be used in SVG?
I believe there are some known SVG elements prohibited from custom element names, which do have a dash

[hesxenon]

Hmmm, what do you mean by "can be used in SVG"? There are no custom svg elements (yet?) (see here point 6.4).

But I would expect them to work as a foreign object, embedded in a svg structure.

Then again there's a way to check for registered custom elements but all in all I think all this just makes the issue more and more complex.

If I understood the code correctly (and mind you, I spent all of 5 minutes skimming over it) changing this line to something like return Object.assign(node, {__h: hastscriptSymbol}) where hastscriptSymbol is a Symbol("hastscript") would simplify these checks greatly.

[wooorm]

can be used in SVG"

This project can be used to generate SVG, with s.

HTML has exceptions for custom element names that overlap with SVG: https://html.spec.whatwg.org/multipage/custom-elements.html#valid-custom-element-name.

see here point 6.4)

I think you meant to link to a spec, but linked to the code here? What is point 6.4 in this code?

there's a way to check for registered custom elements

This code can run on servers. Or ahead of time. So it should work regardless of what is registered.

changing this line to something

hast is a plain vanilla json format; so no symbols.
even if we did that, that would not work with other nodes: people can pass comments and doctypes in.
and even if we checked for that, people should be able to define custom nodes.

---

The definition of “node” here is basically any object with a type field set to a string.
And properties is basically any object with mostly primitive values.
Hence the overlap here.

[hesxenon]

spec

🙈 I'm so sorry, fixed it. Meant to link to the DOM spec.

registered elements

Yeah, hit me 5 min after I wrote it as well...

has is plain JSON

I see, that complicates things...

even if we checked for that people should be able to define custom nodes

Well, they're not able to define custom nodes right now, are they? I get your point, but HTML also disallows certain element names etc... maybe an attribute with spaces in it? Would be json compatible but not feasible as an attribute on a custom element I think?

[wooorm]

Well, they're not able to define custom nodes right now, are they?

One popular example in the wild is MDX, which embeds ESM, JSX, and expressions inside HTML.
But for this project also comments and doctypes are essentially “untreated”.

h('div', {type: 'comment', value: 'x'}) is a simple example

maybe an attribute with spaces in it? Would be json compatible but not feasible as an attribute on a custom element I think?

Can you clarify what you mean?

[wooorm]

Seems to be possible to put arbitrary dash-cased named in the SVG namespace:

> document.body.innerHTML = '<svg><x-y></x-y></svg>'
< "<svg><x-y></x-y></svg>"
> document.body.firstElementChild.namespaceURI
< "http://www.w3.org/2000/svg"
> document.body.firstElementChild.firstElementChild.namespaceURI
< "http://www.w3.org/2000/svg"

[hesxenon]

mdx example

I'm not sure I follow completely, but that example you gave also leads to a node with children instead of properties?

clarification

I was thinking that since Symbol cannot be used we'd just have to find another property that is "unique" to hastscript. And since property names with spaces in them are valid json but are not valid html attributes that could be a solution. I.e. return Object.assign(node, {"hast script": 0}).

Though if you want JSON serializability you'd probably want the tree to be as small as possible, so... also not ideal.

[hesxenon]

dash cased names in the svg namespace

true, but those would never be valid html elements? Is the problem here that, when parsing a node like {tagName: "x-y"}, you could not differentiate between properties and children again? At least not based on the dash in the tag name?

Hmmm, this kinda reminds me of all those discussions around JS types, variadic arguments and "aot" knowledge about arguments^^

Isn't the problem originally caused by wanting to offer an "overload" signature of h(selector, ...children)? Even though it would be a breaking change, requiring attributes would definitely fix it, no?

[wooorm]

I'm not sure I follow completely

h('div', {type: 'msxJsxTextExpression', value: 'props.something'}) is an expression inside a div, equivalent to <div>{props.something}</div> in JSX.

but that example you gave also leads to a node with children instead of properties?

Which is inverse from what you are asking about. You want props instead of a node.
It’s always possible manually to choose “props” to be seen as a “node”, by doing h('div', undefined, whatLooksLikePropsOtherwiseButIsANode).
But it’s not possible to do the inverse: what looks like a node to be passed as props. Yet!

And since property names with spaces in them are valid json but are not valid html attributes that could be a solution

Ahh. Yes, we can allow another field. h('div', {type: 'whatever', isProps: true}). And not actually put it on the resulting properties!
However, at that point, you do not need to use h or pass that prop through: const div = h('div'); div.properties.type = 'whatever' is fine! So is {type: 'element', tagName: 'div', properties: {type: 'whatever'}, children: []}.

but those would never be valid html elements?

The concern of hast is more with what DOMs could be generated by an HTML parser; not with what will eventually work at runtime. Folks could have such HTML files and hast needs to represent it.

Isn't the problem originally caused by wanting to offer an "overload" signature of h(selector, ...children)? Even though it would be a breaking change, requiring attributes would definitely fix it, no?

Yep!

hast must be able to represent it all. hastscript, this project, is a convenience around generating elements.

In regular HTML, type is used for buttons (with specific allowed values), and input, with an arbitrary string value. Hence the exceptions in the code for those.
I was initially thinking of whether we could indeed require props for all dash-cased tag names, as they are likely to be custom.

[hesxenon]

inverse, "you want props instead of a node"

Ah, I see. I didn't realize that hast nodes could also be used like that (even though I did just that a few years ago^^)

no need to use h or pass that prop through

true, I think I'm going to use that in my lib. For my case that solves the issue, thanks :)

Now, for this "doing the inverse"... I'm thinking that if arguments.length > 2, then the second argument would always have to be the properties, no? That way consumers could more easily work around the problem by explicitly calling h(selector, undefined, ...children)? Ofc. it still doesn't eliminate the problem for h(selector, nodeOrMaybePropsDunno)...

[wooorm]

true, I think I'm going to use that in my lib.

What are you doing? The props handling other than the type field here might be needed. As you can see in the code, there’s a lot going on.

Without the props handling and the overload, this project is useless, as you can make elements directly: {type: 'element', tagName: 'c-ustom', properties: {type: 'whatever'}, children: [...]}

then the second argument would always have to be the properties, no

No, you can pass children: h('div', {type: 'text', value: 'asd'}, {type: 'comment', value: 'qwe'}, h('em', 'zxc'))

[hesxenon]

Hey, sorry for the late answer.

What are you doing

Writing a lib to convert jsx to an html string. You can see more here. Fair warning, this project is very much in flux.

Without props handling and overload

I agree as far as the props handling is concerned, but I don't think the overload is necessary for this project to have a purpose. Whether I call h(selector, propsOrChild, ...children) or h(selector, definitelyProps, ...children) makes little difference in practice.

Passing children as second argument

Yeah... you're right again. Well, I don't see many other options then.

[wooorm]

No worries!

Fun!
I don’t think you need this project to do that: the nice-to-have overloads aren’t needed, and the props handling isn’t needed.

The trouble with JSX is that it could be used for anything. Not for HTML per se. If for HTML, then frameworks differ in what they expect too. Some support className, others support class. That’s one trivial example, there are tons. Another is style as objects; and then WebkitTransform or -webkit-transform?
So: you need to have knowledge of what framework the author is writing for. Or give options. The author has to write specifically for your new tool.

Then, JSX is more than just HTML. You have components. And JavaScript code that needs to be evaluated somewhere. How do you want to handle that?

Although, https://github.com/syntax-tree/hastscript/blob/2a7451dc1eb2adc6b07af6efed8a8bdcc8f13758/lib/create-automatic-runtime.js could be used but doesn’t do all you want I believe, and it has the same problem.

I would recommend looking at the existing code across this organization. https://github.com/syntax-tree/hast-util-to-jsx-runtime, https://github.com/syntax-tree/hast-util-from-dom, and also https://github.com/wooorm/property-information and https://github.com/mdx-js/mdx/tree/main/packages/mdx. All depends on whether you’re building something at compile-time or runtime, and what the goals are of the tool.

[hesxenon]

Thanks for the suggestions - before I started writing this lib I did some digging in the unified ecosystem, but found no match for what I wanted :)

I am quite specific in my implementation what kind of JSX I want to allow and am trying to stay as close to HTML as I can, apart from things like allowing CSSStyleDeclarations for the style and Record<string, boolean> | string for the class attribute. It is very much intended to enable the consumer to write HTML-ish JSX with a simple string output.

And yes, come to think of it, I don't think I need this library for that, simply creating the objects should be enough 🤔

[hesxenon]

well, after playing around a bit I think I'm going to stick with this library. While I am currently skipping property handling (which should only be temporary anyway) the children handling is simply convenient to have, and it works fine for my purposes.

[matthewp]

Note that this issue doesn't require custom elements and occurs with <div type="number" value="1"> as well.

[hesxenon]

Yeah, but... when would you want that? That's not valid html, is it?

…

On Thu, Jan 18, 2024, 18:29 Matthew Phillips ***@***.***> wrote: Note that this issue doesn't require custom elements and occurs with <div type="number" value="1"> as well. — Reply to this email directly, view it on GitHub <#21 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACOHZTVJAWHZBRQJC4ANHNLYPFLZJAVCNFSM6AAAAABB2E462OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJYHEYTOMRWGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[wooorm]

@matthewp why are you trying to create that?

[matthewp]

@hesxenon Is it valid HTML, open up your browser and do this:

new DOMParser().parseFromString('<div type="number" value="1"></div>', 'text/html')

And you'll get a DOM node with those attributes. HTML actually doesn't care if you add arbitrary attributes to elements.

@wooorm I don't actually need div to work, just pointing out that it's not a custom element specific problem. My use-case is also custom elements, see here, so adding an exception for dashed names would fix my bug as well.

[wooorm]

"Valid" relating to html is different things. Accepted by the parser and exposed in the DOM are one thing but...

This tool is a small tool that makes it easier for humans to make ASTs.

Why are you using this project, and in way?

[matthewp]

@wooorm In Astro we are using rehype-raw and encountered this issue (with custom elements). The dep tree looks like this:

└─┬ [email protected]
  └─┬ @astrojs/[email protected]
    └─┬ [email protected]
      └─┬ [email protected]
        └─┬ [email protected]
          └── [email protected]

We are directly using rehype-raw which I guess eventually calls down to this code.

[wooorm]

One thing I can think of to solve it is to treat the 2nd argument as parameters when it is ambiguous, and the 3rd argument is an array, and there are exactly 3 arguments.

That's the typical case if you'd do programmatic prop and child building and then call the h function. As is done in both your use cases?

As an aside, why is raw used here in Astro? I thought the mdx jsx stuff was used to parse these things already?

[matthewp]

We don't use the mdx stuff with regular .md.

[wooorm]

Or maybe a toProperties function here does basically everything for all the tools too...

@remcohaszing maybe you have a preference?

[remcohaszing]

Isn't the problem originally caused by wanting to offer an "overload" signature of h(selector, ...children)? Even though it would be a breaking change, requiring attributes would definitely fix it, no?

I think this is the key. There is no way to reliably detect the meaning of the second argument. No matter what you do { type: 'button' } could always mean both attributes and an element.

Personally I would remove this overload entirely, which would of course be semver major.

hastscript/lib/create-h.js

Lines 74 to 77 in 2a7451d

	* @overload
	* @param {string} selector
	* @param {...Child} children
	* @returns {Element}

[wooorm]

I personally quite love doing h('p', 'stuff') and h('div', h('p', 'stuff)). That code looks like it should work fine instead of being treated as properties.

[remcohaszing]

I looks quite nice in examples. I wonder if it’s actually useful in production code too, where parameters are likely some variable, not a hardcoded value.

Personally I like to use hastscript as JSX, but otherwise I use literal objects.

[wooorm]

JSX is one use case but so is calling h manually. E.g., https://github.com/wooorm/wooorm.github.io/blob/d442e863067573a2747064443f793998ed6f5724/generate/render/home.js#L37.
If you only use JSX, you don’t use h directly anyway. The two approaches I mentioned above also solve it. toProperties and locking the string, object, array signature down

[matthewp]

Is the idea of special casing dashed names (custom elements) still ok as a stopgap? This comes up because a popular library Shoelace: https://shoelace.style/ has its own input element and thus mirrors the input API directly: <sl-input value="1" type="number"></sl-input>

[remcohaszing]

I can’t deny the hastscript usage looks pretty neat in https://github.com/wooorm/wooorm.github.io/blob/d442e863067573a2747064443f793998ed6f5724/generate/render/home.js#L37, but is it worth having the meaning of { type: 'button' } ambiguous? I thing explicitly specifying attributes there would be less nice, but still pretty neat.

[wooorm]

I made this project for such use cases. Humans to call a DSL that looks like what it does so they can omit superfluous things. There are several other niceties too, such as h('div#id.class').

Any reason you prefer the breaking change (which is a useful and IMO nice-to-have overload) instead of the other viable alternatives?

Is the idea of special casing dashed names (custom elements) still ok as a stopgap?

Yep, I think that’s fine too!

[hesxenon]

I just want to point out that there are many popular and very convenient ast libraries out there that _do_ force explicit prop passing and are still very convenient and human readable. Your project is yours to do with as you please ofc, and it's a very useful library either way. But I also think removing the overload would be the cleaner option, especially in the long run.

…

On Sat, Jan 20, 2024, 10:11 Titus ***@***.***> wrote: I made this project for such use cases. Humans to call a DSL that looks like what it does so they can omit superfluous things. There are several other niceties too, such as h('div#id.class'). Why do you want to cause a breaking change when there are also like 3 viable alternatives? Anything you don’t like about those alternatives? Is the idea of special casing dashed names (custom elements) still ok as a stopgap? Yep, I think that’s fine too! — Reply to this email directly, view it on GitHub <#21 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACOHZTQVYCQW5UXWYMVYVQTYPOC5FAVCNFSM6AAAAABB2E462OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBSGAZDAMBSGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wooorm]

This issue practically only appeared when props with type and value were passed, which practically only occurs on custom elements. It was seen as a literal node.
That field combination on input and button, the only other combination where it is known to HTML, was already handled.

In most practical cases, it is possible to detect the difference between props and nodes in this overload.
And if it wasn’t possible (void nodes, aka doctypes), it was treated as props.
Except for props on custom elements.

[matthewp]

Thank you @wooorm!