Allow u(type, props, ...children) form of API

Question

Allow u(type, props, ...children) form of API

Josh-Cena opened this issue 3 years ago · 10 comments

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)

Problem

I'm probably doing something cursed, but I'm trying to see if it's able to write unists in JSX. I reproduced the example in the README as:

import { u } from "unist-builder";

const tree = (
  <root>
    <subtree id={1} />
    <subtree id={2}>
      <node>
        <leaf>leaf 1</leaf>
        <leaf>leaf 2</leaf>
      </node>
      <leaf id={3}>leaf 3</leaf>
      <void id={4} />
    </subtree>
  </root>
);

And using u as the JSX factory. Unfortunately, this didn't work, because u only accepts an array of children. In order to make this work, I have to use a single array as children everywhere.

Solution

I propose to make this valid:

var tree = u(
  "root",
  null,
  u("subtree", { id: 1 }),
  u(
    "subtree",
    { id: 2 },
    u("node", null, u("leaf", null, "leaf 1"), u("leaf", null, "leaf 2")),
    u("leaf", { id: 3 }, "leaf 3"),
    u("void", { id: 4 })
  )
);

Because JSX always compiles to type, props, ...children, whenever there are more than three arguments, the rest must all be children, and we can simply collect them into an array.

Alternatives

Do not use JSX for unist? 😄

I'm really curious if I'm able to make it work, though

Answer 1 · 2022-05-18T08:42:10.000Z

Perhaps better to add support for the modern automatic runtime, like https://github.com/syntax-tree/hastscript#jsx and xastscript?

Answer 2 · 2022-05-18T08:44:58.000Z

Can we do both? Hastscript also supports both the h(selector?[, properties][, …children]) API and automatic runtime.

Answer 3 · 2022-10-27T16:11:48.000Z

I’d prefer only an automatic JSX runtime. I didn’t really like it myself when they announced it, but with my MDX work, I now really really prefer it over the classic runtime.
I’m open to a classic runtime, but perhaps we can tell people to import something else that isn’t u from unist-builder?

Main important points for me:

I don’t want to do breaking changes to the API only if it’s “nice for a couple folks”
I don’t want to weigh regular users down with more code
I’m open to (experimenting with) an improved JSX experience

If those are met, I’m 👍

Answer 4 · 2022-10-28T09:07:03.000Z

I don’t see any problem breaking changes when adding support to a classic runtime. Currently the API accepts a string or an array of objects as its third argument.

assert.deepEqual(
  <root>
    string
  </root>,
  { type: "root", value: "string" }
)

assert.deepEqual(
  <root>
    <child />
  </root>,
  { type: "root", children: [ { type: 'child' } ] }
)

assert.deepEqual(
  <root>
    <child />
    <child />
  </root>,
  { type: "root", children: [ { type: 'child' }, { type: 'child' } ] }
)

assert.throws(
  () => (
    <root>
      string
      <child />
    </root>
  ),
  'Expected either a string or an array of nodes, got invalid array'
)

Answer 5 · 2022-10-28T09:11:31.000Z

Should fragments get a special meaning?

Answer 6 · 2022-10-28T09:24:36.000Z

JSX in the classic runtime only accepts children (Array<*>). Where * is an actual node or the result value of an expression, which is typically turned into a text node.
JSX only passes children as variadic parameters: not an array of children. So we can’t know whether a node is supposed to be a parent, but doesn’t have children.

JSX will pass multiple strings. We don’t know what to do with strings (how to handle b in <>a{}b</>)
JSX will pass strings after the first. We don’t know what do do with non-first strings (how to handle b in <><x />b</>)
JSX will pass numbers, null, undefined, and other values from expressions through as children. We do not know how to handle those.
JSX will not pass anything for no children. The intention could be an empty parent or a void node (<x />)
We indeed do not know what a fragment is.

unist accepts anything. For JSX, everything is a parent, and text nodes are a certain thing. There’s a mismatch there that can’t be solved without domain knowledge. That‘s why hast/mdast/xast/etc can work well with JSX, but I don’t think this can.

Answer 7 · 2022-10-28T10:01:10.000Z

JSX will pass multiple strings. We don’t know what to do with strings (how to handle b in <>a{}b</>)

JSX will pass strings after the first. We don’t know what do do with non-first strings (how to handle b in <><x />b</>)

You’re right, my bad. Still this seems fine?

u('root', null, 'value')

assert.deepEqual(
  u('root', null, u('child', null), u('child', null)),
  u('root', null, [u('child', null), u('child', null)])
)

assert.throws(() => {
  u('root', null, 'value', 'value?')
}, 'Unexpected arguments, one string value is allowed, got: "value?"')

JSX will pass numbers, null, undefined, and other values from expressions through as children. We do not know how to handle those.

I don’t see any problems here. One could also pass plain values when writing plain JavaScript. This can be checked both at runtime and using TypeScript.

JSX will not pass anything for no children. The intention could be an empty parent or a void node (<x />)

Isn’t this already supported? From the readme:

u('void', {id: 4})

unist accepts anything. For JSX, everything is a parent, and text nodes are a certain thing. There’s a mismatch there that can’t be solved without domain knowledge. That‘s why hast/mdast/xast/etc can work well with JSX, but I don’t think this can.

I think it could work. I definitely see the similarities between JSX and unist. Yes, JSX has text nodes, but that doesn’t mean should allow them. We could disallow them using TypeScript, even though a single string child would still technically work.

I’m not sure it should be supported though. I like that it’s supported by hastscript and xastscript, because JSX looks a lot like HTML and XML. For this package it seems a bit more like a coincidence.

Answer 8 · 2022-10-28T10:25:45.000Z

Isn’t this already supported? From the readme:

It could be fine. But, according to unist there are three shapes of nodes:

void (it must not have value and children fields).
literal (it must have a value field)
parent (it must have a children field)

Users will try to pass <x />, <x>stuff</x>, and <x><y /></x>, and expect them to generate a certain node interface, but it will generate three different node interfaces, two of them then, according the the unist rules, will be invalid.
I think it’s too easy to generate invalid unist trees like this.
And I don‘t see a way around it.

Still this seems fine?

A root that has a value field and does not have a children field is not fine according to mdast/hast/xast. Throwing on multiple strings is possible though. But then still, you’re generating a root with a value field.

Answer 9 · 2023-01-23T13:30:27.000Z

I think JSX is a bad idea at this abstraction (see previous comment).

As for accepting a node instead of the current array of nodes or the value, that would get complex if props are also omitted, because a node matches the props interface, however, we can be sure that a node has a type: string field, whereas that would not make sense in props (because an explicit type: string is passed as the first parameter):

u('element', {type: 'text', value: 'whatever'})

Accepting ...children is also ambiguous if children is empty. That signature indicates that a parent has to be build, but that can’t be inferred, instead a void will be made:

const children = []
u('element', {whatever: true}, ...children) // {type: 'element', whatever: true}
u('element', ...children) // {type: 'element'}

As it’s all so ambiguous, so many edge cases that an author needs to be aware of, I’m inclined to just not do it.

What do you think @Josh-Cena? Haven‘t heard from you in a while :)

Answer 10 · 2023-02-06T17:57:29.000Z

Ok closing, I’m not really sure about the trade offs!