hastscript icon indicating copy to clipboard operation
hastscript copied to clipboard

Allow functional components in JSX

Open alecmev opened this issue 2 years ago • 11 comments

Initial checklist

Problem

I'm using TypeScript. I'd like to do something like this:

const Header = ({ children }: { children: HChild }) => <h1 className="foo">{children}</h1>
const Section = <section><Header>Bar</Header></section>

But this line makes TypeScript reject it:

https://github.com/syntax-tree/hastscript/blob/2f8c133920fa57405815de3f3cdc21d1bc892444/lib/jsx-automatic.d.ts#L9-L12

With one of the following errors (depending on the amount of children provided):

This JSX tag's 'children' prop expects a single child of type 'never', but multiple children were provided.
This JSX tag's 'children' prop expects type 'never' which requires multiple children, but only a single child was provided.

Solution

Allow functional components by removing type IntrinsicAttributes = never. Or is there a reason they aren't?

Alternatives

This works, but it's clunky:

const Header = (children: HChild) => <h1 className="foo">{children}</h1>
const Section = <section>{Header("Bar")}</section>

alecmev avatar Jul 27 '22 21:07 alecmev

Did you see the comment above that line?

It is intentional that components cannot be passed in the types, and hence that that line is there, because components are not supported in hastscript.

How are you using components currently, if they aren’t supported? 🤔

See also: https://github.com/syntax-tree/unist-builder/issues/11

wooorm avatar Jul 28 '22 08:07 wooorm

Sorry for my ignorance, for some reason I assumed <Header>Bar</Header> gets transpiled into Header({ children: "Bar" }), while in reality it's _jsx(Header, { children: "Bar" }), so it won't work until h supports functions as selector.

Is that something that you would accept a PR for? At a glance, a simple pass-through should work, as suggested here:

} else if (typeof selector === "function") {
  return selector({ ...properties, children: value });
} else {

Inserted here:

https://github.com/syntax-tree/hastscript/blob/2f8c133920fa57405815de3f3cdc21d1bc892444/lib/core.js#L68-L70

No need to handle properties or children, it's component's responsibility. Or am I missing something?

alecmev avatar Jul 28 '22 10:07 alecmev

Yes, I believe that that is the addition.

However, what “is” a component? Do components even make sense here? What about “class” components? What about other lifetime things? Should children be a field, instead of another parameter? Should components return elements? Could they return other things?

wooorm avatar Jul 28 '22 16:07 wooorm

@alecmev Friendly ping! Do you have answers to my questions?

wooorm avatar Aug 23 '22 19:08 wooorm

Sorry! Got a bit of a Github backlog.

However, what “is” a component?

I'd say it's just syntactic sugar for an element factory. So that instead of this:

<section>{Header(<div>foo</div>, { isFoo: true })}</section>

I can write this:

<section><Header isFoo><div>foo</div></Header></section>

Do components even make sense here?

Make sense? I think so, lots of people are familiar with the concept of functional components, and it appears to fit in, since it looks and smells like React or any other JSX-friendly presentation layer library.

Is this a killer feature? Probably not. Composition still works, it just doesn't play by JSX rules (so the function signature can be whatever you want). I might be forgetting something, but it's not coming to me right now, been away from React for too long.

What about “class” components? What about other lifetime things?

Out of scope, I think. There's no state. "Components" return the HResult and they're done.

Should children be a field, instead of another parameter?

This is up to the spec and transpilers, I think? So there's no choice, children is just a special prop. I might be wrong.

Should components return elements? Could they return other things?

They must return an HResult. And maybe an array of HResult? Need to investigate.

Another reason to have this is because people writing JavaScript will try this, being familiar with React. They'll see that it doesn't work, but it might take a bit to understand why.


Sorry, my answers are superficial and possibly inaccurate, but I thought this could still be useful. I can go more in-depth if you start to really consider implementing this. Also, seemed like @remcohaszing might be able to add something?

alecmev avatar Aug 23 '22 19:08 alecmev

I previously assumed that we’d add component support to h:

h(Component, props?, ...Array<Child|string|Array<Child|string>>)

But we could also just add it to the automatic runtime: https://github.com/syntax-tree/hastscript/blob/2f8c133920fa57405815de3f3cdc21d1bc892444/lib/runtime.js#L31

Because the main Q seems to be: “JSX with hastscript looks like JSX with React, so I expected components to work too”.

This is up to the spec and transpilers, I think? So there's no choice, children is just a special prop. I might be wrong.

Our _jsx function must indeed handle props.children in the automatic runtime. In the classic runtime, this wasn’t defined. Still, in _jsx of our automatic runtime, we take it out: https://github.com/syntax-tree/hastscript/blob/2f8c133920fa57405815de3f3cdc21d1bc892444/lib/runtime.js#L32.

wooorm avatar Aug 24 '22 07:08 wooorm

I can see why this is useful. It’s mostly semantic. Because of how the automatic runtime and TypeScript types, children should be passed as an attribute named children.

I also think children should be coerced into a flat array. This is what makes usage more than just semantic.

function FancyHeader({ children }) {
  // children is always an array of element or text nodes here.
}

<FancyHeader>
  <div />
  <div />
</FancyHeader>
<FancyHeader>
  <div />
</FancyHeader>
<FancyHeader>
  {[<div />]}
</FancyHeader>
<FancyHeader>
  <>
    <div />
  </>
</FancyHeader>
<FancyHeader children={<div />} />
<FancyHeader children="Hello" />
<FancyHeader>
  Hello
</FancyHeader>

I also wonder if this would cause any issues in xastscript.

remcohaszing avatar Aug 25 '22 09:08 remcohaszing

xastscript

What problems are you thinking off?

--

@alecmev I think I’d be open to a PR! Interested in working on it?

wooorm avatar Aug 25 '22 17:08 wooorm

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

github-actions[bot] avatar Aug 25 '22 17:08 github-actions[bot]

@alecmev Friendly ping! Something you’d like to work on?

wooorm avatar Oct 12 '22 18:10 wooorm

@alecmev Ping! 🔔 :)

wooorm avatar Oct 27 '22 15:10 wooorm

xastscript

What problems are you thinking off?

About the fact that children is a valid XML attribute. Although I don’t really think it affects this issue.

remcohaszing avatar Oct 28 '22 09:10 remcohaszing

@wooorm Sorry for the silence again, I remember about this, simply have more pressing tasks lately. I'll get around to this eventually, but if somebody wants to pick this up - please do go ahead!

alecmev avatar Dec 22 '22 17:12 alecmev

@alecmev if this isn’t really needed for you, we can also close it :) I think it’s an interesting idea, and I don’t see big downsides, but also: if nobody really practically needs it (yet), it’s easier to maintain less code!

wooorm avatar Feb 06 '23 18:02 wooorm

Let's keep this open. I'd really like to see this feature as well.

I think hastscript + this feature + mdx + node --watch / tsx watch could yield a pretty good static site generation experience for minimalistic websites.

If nobody else fixes this issue, I will some day.

remcohaszing avatar Feb 06 '23 21:02 remcohaszing

@wooorm I get the appeal of "GitHub Zero" and less stuff to maintain :wink: Okay for me if you close it! I have this in my personal backlog.

alecmev avatar Feb 06 '23 21:02 alecmev

If nobody else fixes this issue, I will some day.

You are welcome to do so.

I would really like for someone to actually need and use this feature. Implementing features that nobody uses is just scope creep and it’ll likely be broken for your eventual use case, if we don’t try to build that use case in tandem: see this thread, it’s unclear how it would work.

As this isn’t actionable currently, I think it should be closed. Closing this doesn’t mean it will never be actionable

wooorm avatar Feb 07 '23 11:02 wooorm

I'm using JSX in Deno, and was using Preact (and preact-render-to-string), but I was curious as to whether hastscript would be suitable instead. At first the lack of function components tripped me and the incompatible TS types, but I've eventually found a work-around, by wrapping the jsx function with my own, and supplying my own JSX type:

// deno-lint-ignore-file no-explicit-any
import * as r from "hastscript/jsx-runtime";
import type { Child, Properties, Result } from "hastscript";

export const jsx = (type: any, props: any) => {
  if (typeof type === "function") {
    return type(props);
  } else {
    return r.jsx(type, props);
  }
};

export const jsxs = jsx;
export const jsxDEV = jsx;
export const Fragment = r.Fragment;

// deno-lint-ignore no-namespace
export namespace JSX {
  export type Element = Result;

  export interface IntrinsicAttributes {
    key?: any;
  }

  export interface IntrinsicElements {
    [name: string]:
      | Properties
      | {
        children?: Child;
      };
  }

  export interface ElementChildrenAttribute {
    children?: any;
  }
}

I think this could be a fairly simple change to the hastscript runtime to support functions, and provide a better (least surprise) experience for any devs trying to use hastscript for jsx.

This and hast-util-to-html now works for me as a drop-in replacement of preact & preact-render-to-string.

jollytoad avatar Mar 16 '23 17:03 jollytoad

This is very similar to what I had in mind, except I would also like to flatten children before passing them to type()

remcohaszing avatar Mar 16 '23 17:03 remcohaszing

It’s still open, but it needs someone who a) actually needs this/is going to use this, and b) it will require making several decisions, which need to be discussed.

wooorm avatar Mar 16 '23 18:03 wooorm

Closing, it’s perhaps of interest, but needs someone who wants to work on it and actually use it, instead of a theoretical nice to have IMO! Thanks all :)

wooorm avatar Jun 21 '23 17:06 wooorm

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

github-actions[bot] avatar Jun 21 '23 17:06 github-actions[bot]

Funny, that this was closed yesterday.... I actually discovered this today, and I have a real usecase: a vanilla HTML templating library (no stateful components, just one-way functions that generate html). I had been using my own html library and corresponding jsx-transform.

I was thinking to drop that in favor of using hastscript in order to benefit from the entire rehype ecosystem, but then I hit this. I can submit a PR if it would be accepted.

cowboyd avatar Jun 22 '23 15:06 cowboyd

Yeah feel free to work on it in a pr! See the above thread, I believe there are some points to dicuss/figure out!

wooorm avatar Jun 22 '23 15:06 wooorm

@wooorm Ok, I created a PR that implements the transform for the "automatic" runtime. https://github.com/syntax-tree/hastscript/pull/19

I'm not quite certain what should be done about the "classic" runtime though since it's basically a pass-through.

cowboyd avatar Jun 23 '23 10:06 cowboyd

While the classic runtime remains to be figured out, I plugged in the automatic JSX transform into my website and it worked flawlessly.

cowboyd avatar Jun 23 '23 12:06 cowboyd

I ended up creating a very simple jsx-runtime, hastx that emits HAST directly instead of passing it through hastscript. This is very similar to what @jollytoad suggested as his workaround except that it creates the HAST data structures directly rather than relying on the h() function.

Also, it flatten's children from Root nodes into the tree per @remcohaszing's suggestion (but it does not yet flatten children for function types).

  • https://deno.land/x/hastx
  • https://npmjs.org/package/hastx

Caveats

  • It only supports the modern JSX transform, and not "classic" jsx factory a-la React.createElement
  • It does not do any type schema checking on HTML/SVG element attributes (e.g. <a wat={true} /> will be just fine).

Hopefully this could serve as a blueprint to eventually roll into hastscript proper, but was just enough to satisfy my immediate use-case perfectly.

cowboyd avatar Jul 03 '23 07:07 cowboyd