[hyperscript] The selector parser is very loose for classes, ids and tag names

[pygy]

We use selectorParser = /(?:(^|#|\.)([^#\.\[\]]+))|.../ to parse the hyperscript selectors.

As mentioned by @cavemansspa This can be nice for multiple classes: m('div.ui grey inverted segment').attrs => { className: 'ui grey inverted segment' }, but problematic for tag names since it can lead to crashes.

While Chrome prints the problem string in the error Message, Firefox and Safari don't, which is annoying. The stack trace is not helpful since it is all in m.render, it doesn't point at the error in user code.

We could add an additional check for tag names in the compileSelector() function, it would be a one time cost per selector string.

[dead-claudia]

@pygy Here's my thoughts:

• The CSS selector div.ui.grey checks one element with two classes. div.ui grey checks for <grey> elements within a <div class="ui">.
• IDs aren't supposed to have spaces in them, although browsers are supposed to tolerate them. Instead, they're supposed to follow the same grammar class names have. CSS can't select IDs with spaces in them anyways, so it's poor style either way.
• Engines are supposed to throw when you create an element with an invalid tag name - that's required per spec.

I think the ideal way to handle this is to just not permit such spaces in selectors, and throw early if one exists. This could be as easy as just adding a separate /\s/.test(value) check to harass people to fix their selectors from a more useful spot. The alternative here is to admit them and either handle them as parent/child paths* or ignore them, but either option would result in some very unhelpful behavior since we're pretty much covering up what's likely just the user mis-using the framework (we're emulating CSS <compound-selector>s).

* I recall seeing a while back an old v0.2 Mithril utility that allowed h("div > span > .foo.bar") or something similar which did this, but I can't remember the name of it for the life of me.

[orbitbot]

re: the footnote, that was probably mithril-query which IIRC was built on cssauron or something like that.

[dead-claudia]

@orbitbot Specifically, something that desugared h("div > span > .foo.bar") to m("div", m("span", m(".foo.bar"))). It might be that and I might be misremembering what it did, but I thought it was something else.

[MKRhere]

I know this is low-priority and unlikely to be an issue for real use-cases, but adding my thoughts here on spaces in selectors.

m("div class")
//-> { tag: "div class" }
// invalid

m("div .class class-2")
//-> { tag: "div", class: "class class-2" }
// Mithril tolerates this

m("div #id id-part")
//-> { tag: "div", id: "id id-part" }
// tolerated in browsers

m("div #id .class #id-2 .class-2")
//-> { tag: "div" id: "id-2" class: "class  class-2" }
// Ignores id and is valid, but notice the extra space before class-2

I'm not sure what the right behaviour should be. I've been using Mithril for a long time, but I'm writing my own little lib and checked what Mithril's behaviour is on different things and how I should support them. My parser is also loose, but it's not meant to be a strict CSS selector subset. I'm allowing IDs to have space, and anything else that doesn't have a dot or # becomes a classname. This is because I'll wrap them in helper syntax, similar to my other lib: mithril-toolset

The following are valid in my case:

h("tag class")
h("tag #id class")
h("tag .class #id id-part")
h("tag .class #id id-part .class-2 class-3")

Multiple IDs error instead of passing through.