Skip to content

Add support using x as a JSX pragma #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 3, 2020
Merged

Add support using x as a JSX pragma #4

merged 4 commits into from
Nov 3, 2020

Conversation

wooorm
Copy link
Member

@wooorm wooorm commented Nov 2, 2020

This PR tests that xastscript can be used as the pragma for JSX with bublé
and babel.

Code-wise, this adds support for using x to generate root nodes.
This is done by omitting the tag name (like so: x(), x(null, 'child')).
Previously, omitting a name resulted in an exception.

Another aspect of supporting JSX is supporting fragments as children.
As fragments yield root nodes, we unravel them and use only their children.
While this could be seen a change, xast prohibits roots occurring in nodes,
so the unraveling instead fixes what would otherwise be a broken tree.

Related to: GH-3.

/cc @remcohaszing, @ChristianMurphy

@wooorm wooorm added 🗄 area/interface This affects the public interface 🧒 semver/minor This is backwards-compatible change 🙉 open/needs-info This needs some more info 💬 type/discussion This is a request for comments labels Nov 2, 2020
Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@wooorm wooorm marked this pull request as ready for review November 2, 2020 16:39
@wooorm wooorm added 🙆 yes/confirmed This is confirmed and ready to be worked on 🦋 type/enhancement This is great to have and removed 💬 type/discussion This is a request for comments 🙉 open/needs-info This needs some more info labels Nov 2, 2020
This PR tests that `xastscript` can be used as the pragma for JSX with bublé
and babel.

Code-wise, this adds support for using `x` to generate root nodes.
This is done by omitting the tag name (like so: `x()`, `x(null, 'child')`).
Previously, omitting a `name` resulted in an exception.

Another aspect of supporting JSX is supporting fragments as children.
As fragments yield root nodes, we unravel them and use only their children.
While this could be seen a change, xast prohibits roots occurring in nodes,
so the unraveling instead fixes what would otherwise be a broken tree.

Related to: GH-3.
@wooorm wooorm merged commit db94972 into main Nov 3, 2020
@wooorm wooorm deleted the jsx branch November 3, 2020 07:50
@remcohaszing
Copy link
Member

remcohaszing commented Nov 3, 2020

This happened really fast.

I think the idea of using null to define fragments is clever, although it doesn’t work with TypeScript and I doubt if using this to define the root element is the most useful use case.

What about adding support for adding actual functional components, so these can be used to generated non-element nodes.

I.e. the root node:

Implementation:

function Root(props, ...children) {
  return { type: 'root', children };
}

Usage:

/** @jsx h */
const { Root, x } = require('hastscript')

module.exports = (
  <Root>
    <foo>
      <bar />
    </foo>
  </Root>
)

This way more types of nodes can be defined as well, i.e.:

/** @jsx h */
const { Comment, Doctype, Instruction, Root, x } = require('hastscript')

module.exports = (
  <Root>
    <Doctype name="" public="" system="" />
    <Instruction name="" />
    <Comment>Hello</Comment>
    <foo>
      <bar />
    </foo>
  </Root>
)

The implementation in x would be something along the lines of the following, where children is flattened:

if (typeof tag === 'function') {
  return tag(attributes, ...children)
}

I think the most common use case for fragments is to do something like this:

/** @jsx h */
const { Comment, Fragment, Root, x } = require('hastscript')

module.exports = (
  <Root>
    <Doctype name="" public="" system="" />
    <Instruction name="" />
    <people>
      {people.map(person => (
        <>
          <name>{person.name}</name>
          <age>{person.age}</age>
        </>
      ))}
    </people>
  </Root>
)

@wooorm
Copy link
Member Author

wooorm commented Nov 3, 2020

This happened really fast.

Yes, sorry!

although it doesn’t work with TypeScript

Would any other symbol work? React.Fragment is also just a value. Because undefined is handled too, and if this were CJS or we’d explicitly export Root as undefined or null, your first usage example works.

and I doubt if using this to define the root element is the most useful use case.

The changes in the API introduced in this PR are a two part act: yes, roots can be made, but roots are also unraveled. So fragments in your last example also work: https://github.com/syntax-tree/xastscript/pull/4/files#diff-f5b3878f8f75d9cd3df39252cba892d4689833207338622ddb3ae7ecb2c6ef17R106

What about adding support for adding actual functional components, so these can be used to generated non-element nodes.

This to me sounds like a different feature that could be added on top. Or not. As JSX handled Capitalized and members.of.objects as identifiers instead of strings, they can all be used already: https://github.com/syntax-tree/xastscript/pull/4/files#diff-f5b3878f8f75d9cd3df39252cba892d4689833207338622ddb3ae7ecb2c6ef17R44, if they resolve to valid values for name.

What is the benefit of adding “components”?

This way more types of nodes can be defined as well, i.e.:

Those nodes aren’t used a lot, is this an actual need? React also doesn’t support comments. Nodes can also already be passed as children: https://github.com/syntax-tree/xastscript#jsx.

If we’d just want to add other nodes, we could also support “templates”:

x.Comment = {type: 'comment'}
x.Doctype = {type: 'doctype'}
x.Fragment = undefined
x.Root = undefined

function x(name, attributes) {
  var node =
    name == null
      ? {type: 'root', children: []}
      : typeof name === 'object'
      ? Object.assign({}, name)
      : {type: 'element', name: name, attributes: {}, children: []}

  // ... and handle `attributes` as direct props instead of `node.attributes`
}

const { Comment, Doctype, Fragment, Root, x } = require('xastscript')

module.exports = (
  <Root>
    <Doctype name="" public="" system="" />
    <Instruction name="" />
    <people>
      {
        // ...
      }
    </people>
  </Root>
)

@remcohaszing
Copy link
Member

although it doesn’t work with TypeScript

Would any other symbol work? React.Fragment is also just a value. Because undefined is handled too, and if this were CJS or we’d explicitly export Root as undefined or null, your first usage example works.

Apparently the React TypeScript definitions don’t match the React code here. I think your solution below to define Fragment / Root as undefined is pretty awesome. If this is then “wrongfully” typed as a function returning xast.Root, this should work with TypeScript.

I do love the idea fragments don’t need to be imported. :)

and I doubt if using this to define the root element is the most useful use case.

The changes in the API introduced in this PR are a two part act: yes, roots can be made, but roots are also unraveled. So fragments in your last example also work: https://github.com/syntax-tree/xastscript/pull/4/files#diff-f5b3878f8f75d9cd3df39252cba892d4689833207338622ddb3ae7ecb2c6ef17R106

I didn’t know that. That’s awesome! Never mind any of my objections then. :)

What about adding support for adding actual functional components, so these can be used to generated non-element nodes.

This to me sounds like a different feature that could be added on top. Or not. As JSX handled Capitalized and members.of.objects as identifiers instead of strings, they can all be used already: https://github.com/syntax-tree/xastscript/pull/4/files#diff-f5b3878f8f75d9cd3df39252cba892d4689833207338622ddb3ae7ecb2c6ef17R44, if they resolve to valid values for name.

What is the benefit of adding “components”?

The main reason for this was to define Root, Comment, or Doctype as functional components. Your “templates” solution works as well, although it does mean type definitions will have to be technically incorrect to support their usage in TypeScript.

It still may be useful to support components, so the XML tree can be composed using JSX syntax.

This way more types of nodes can be defined as well, i.e.:

Those nodes aren’t used a lot, is this an actual need? React also doesn’t support comments. Nodes can also already be passed as children: https://github.com/syntax-tree/xastscript#jsx.

This was basically a continuation on the thought Fragments must be functions.

@wooorm
Copy link
Member Author

wooorm commented Nov 3, 2020

Apparently the React TypeScript definitions don’t match the React code here. I think your solution below to define Fragment / Root as undefined is pretty awesome. If this is then “wrongfully” typed as a function returning xast.Root, this should work with TypeScript.

and:

This was basically a continuation on the thought Fragments must be functions.

That seems weird? I thought this was pretty common? Such as: https://stackoverflow.com/a/33471928

It still may be useful to support components, so the XML tree can be composed using JSX syntax.

Yeah, I wonder how that’d work. If it would be useful.
Functions may be nicer than templates btw, as those could have deep props, and a factory is just easier than adding deep clone support.

@wooorm wooorm added ⛵️ status/released and removed 🙆 yes/confirmed This is confirmed and ready to be worked on labels Nov 6, 2020
wooorm added a commit to syntax-tree/hastscript that referenced this pull request Nov 6, 2020
This change tests that `hastscript` can be used as the pragma for JSX with bublé
and babel.

Code-wise, this adds support for using `h` to generate root nodes.
This is done by omitting the tag name (like so: `h()`, `h(null, 'child')`).
Previously, omitting a `name` resulted in the default element to be created
(`div` for `h`, `g` for `s`).

This change thus is a breaking change.
The old behavior is still available when passing an empty string: `h('')`.

Another aspect of supporting JSX is supporting fragments as children.
As fragments yield root nodes, we unravel them and use only their children.
While this could be seen a change, hast prohibits roots occurring in nodes,
so the unraveling instead fixes what would otherwise be a broken tree.

Related to: syntax-tree/xastscript#3.
Related to: syntax-tree/xastscript#4.
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
@wooorm wooorm mentioned this pull request Apr 20, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

3 participants