Is it a normal behavior to have all element properties stringified (through hast-to-hyperscript) ?

[medfreeman]

Hi, thanks for your work !!

here's a bit of context.

I'm developing a common mark generic extensions plugin, supporting this kind of syntax:
!Element[content](argument){ properties }

I'm using !Icon[My-tooltip](my-icon){ floating } for testing

My remark inline tokenizer returns this node:

{
  type: 'extension',
  data: {
    hName: 'Icon',
    hProperties: {
      tooltip: 'My-tooltip',
      icon: 'my-icon'
      floating: true
    },
  }
})

Notice the boolean property named floating.

I can properly have my corresponding react component TooltipIcon render with the following snippet (es6):

import remark from "remark"
import reactRenderer from "remark-react"
import merge from "deepmerge"
import sanitizeGhSchema from "hast-util-sanitize/lib/github.json"

import TooltipIcon from "../TooltipIcon"
import genericExtensions from "./remark-commonmark-generic-extensions.js"

remark()
    .use(genericExtensions, {
      elements: {
        Icon: {
          attributeMap: {
            content: "tooltip",
            argument: "icon",
          },
          attributeDefaultValues: {
            floating: true,
          }
        }
      }
    })
    .use(reactRenderer, {
      sanitize: merge(sanitizeGhSchema, {
        tagNames: [
          "Icon",
        ],
        attributes: {
          Icon: [
            "className",
            "tooltip",
            "icon",
            "floating",
          ],
        },
      }),
      remarkReactComponents: {
        Icon: TooltipIcon
      },
    })
    .processSync(body, {
      commonmark: true,
    })

Whereas my floating property is effectively boolean inside the HAST tree generated by remark, it is stringified by hast-to-hyperscript at this line, called here in remark-react.

In order to avoid a react PropTypes type warning, i'm actually forced to also allow String in addition to Boolean propTypes for the floating property inside my component. I then coerce the floating property back to boolean, in order for the subcomponent (which requires floating to be boolean) to be happy.

Here's my TooltipIcon component:

import React, { PropTypes } from "react"
import IconButton from "react-toolbox/lib/button"
import Tooltip from "react-toolbox/lib/tooltip"

const TooltipButton = Tooltip(IconButton)

const TooltipIcon = props => {

  const { floating, ...otherProps } = props
  if (floating === "true") {
    otherProps.floating = true
  }

  return (
    <TooltipButton
      { ...otherProps }
    />
  )
}

TooltipIcon.propTypes = {
  floating: PropTypes.oneOfType([
    PropTypes.bool,
    PropTypes.string,
  ]),
  tooltip: PropTypes.any,
  theme: PropTypes.object,
}

export default TooltipIcon

I hope you get the general idea, and if you can tell if it's a requirement to have every property stringified.
Because in this case only String properties can be passed to React if i'm not mistaken.

[wooorm]

Hi @medfreeman, thanks for the thorough details!

Am I correct in assuming this is an issue with hast-to-hyperscript? I think we need to change code in there.

For a solution, I’m thinking hast-to-hyperscript should not coerce to strings if dealing with a React tree. Do you know if React’s VDOM accepts non-strings in normal elements too, or just for components?

[medfreeman]

@wooorm Thanks for your really quick answer !

I wasn't sure if it was an abnormal behavior in hast-to-hyperscript or remark-react, since i don't understand all the finer points, but that makes sense.

About react vdom, i'm looking into this right now.

[medfreeman]

About React's VDOM and non-strings in normal html elements, i'm not sure of the exact answer, but i'm getting a pretty good idea of how the thing works after a few hours of reading doc / looking at react's code.
However i'm just too tired to express this properly, i'll make you a nice bullet list with code references tomorrow.

[wooorm]

Please go to sleep! 💤 This can wait!

[medfreeman]

Do you know if React’s VDOM accepts non-strings in normal elements too, or just for components?

The answer is basically yes, but the processing differs depending on the node / attribute:

• style, dangerouslySetInnerHTML, children, __html, suppressContentEditableWarning are processed separately, i won't go into the details, info here (code ref)
• properties corresponding to event handlers are bound to the passed function
• if it is on a custom component (i.e. polymer, that contains a dash in its tagname) and the property isn't reserved and doesn't contain invalid chars, it goes through setValueForAttribute, is stringified ('' + value) or removed if value is null
• if not the above, and attribute is in this list (code ref) or a data- or aria- attribute, it goes through setValueForProperty where:
  • if prop has mutationMethod, for now that only applies to the value property and has a value, it is stringified, with exceptions for number validation in input type fields (for browser validation/focus/blur reasons)
  • if prop value should be ignored, it is unset
  • if it has a MUST_USE_PROPERTY flag (ref. here), it is passed as is to the corresponding dom object property
  • if not one of the above, and:
    • element namespaced (i.e. svg) -> namespaced and stringified
    • is boolean: AND true ->set attribute to empty string / OTHER value -> stringified
• The "unknown" properties are dropped for now, but should be passed on from ~v.17, see Allow custom (nonstandard) attributes. facebook/react#140, current PR Allow custom attributes by default facebook/react#7311

References in order:

Differences with html attributes

All Supported HTML Attributes

updateDOMProperties

setValueForProperty and setValueForAttribute reference

shouldIgnoreValue

propertyInfo initialization

propertyInfo config definition

mutator function

facebook/react#140
facebook/react#7311

[medfreeman]

I don't know if everything will be of use to you, but i hope i've been concise enough !

[wooorm]

This is really awesome! I’m working on better props tests, and support for this, but that may take a while depending on how it goes (I’ve got a busy week unfortunately)!

[medfreeman]

Thanks ! Can you explain what part of this hast-to-hyperscript has to support ?
Everything ?
Just verifying variable types ?
In theory couldn't just everything pass through ?
Sorry but i don't exactly understand the ins and outs of hyperscript, and if there is sanitizing work to do.
If you can make a resume, i can try to make a PR.

[wooorm]

So the current tests were focussed on creating the same tree through hast-to-hyperscript and the original h (in this case React.createElement).

However, I found that to be a bit buggy when I was rewriting the tests: now I’m going:

1. hast -> hast-to-hyperscript(React.createElement) -> renderToStaticMarkup -> rehype-parse
2. hast -> rehype-stringify -> rehype-parse
   ...and comparing the two trees for deep equality. I found some bugs and am working on fixing them!

[medfreeman]

ok i understand ! thanks for your work !! i'll use coercition in the mean time ! do not hesitate to ask if you need a bit of help, i can certainly spare a few hours during the next days.

[wooorm]

I’ve pushed a new version to hast-to-hyperscript, 3.0.0, haven’t had time to update this though!

[medfreeman]

Should i just make a PR updating hast-to-hyperscript ?

[wooorm]

Yeah!