Support JSX-Fragments with custom jsxFactory

[marvinhagemeister]

Really love the new JSX-Fragment syntax. It's great 💯

I was wondering if we could enable it for custom jsxFactory in the future. I'd really love to use this feature outside of react 👍

Use cases
Basically any non-react framework that works with jsx (mithril, preact, custom ones...).

Current workaround:

// my-library.ts
import { h, Fragment } from "./my-library";

export const React = {
  createElement: h,
  Fragment,
}

[RyanCavanaugh]

@uniqueiniquity can you investigate how other frameworks emit this to see what we should do to support this?

[dyst5422]

I have another use case of generating XML files where including extra tags is not an option.

I have no idea how this may be implemented, but the need is great enough for our team that I would be willing to spend some time on it.

[uniqueiniquity]

After a brief search, it seems that the only React alternative (out of Mithril, Preact, Virtual-DOM, and Deku) that implements fragments is Mithril, which uses m.fragment(attrs, children) instread of React's approach.

However, I think the key observation is here: preactjs/preact#946 (comment)
By providing a way to specify an alternative to React.Fragment like Babel does, we both open the possibility for frameworks to provide an implementation as well as for people today to use span or similar to patch the behavior. So it seems worth considering to me.

[mikelnrd]

Hi. Also implementing a framework needing a custom tsx implementation supporting JSX Fragments. I think this could be best solved with a compiler option to allow replacing React.Fragment with a user-defined string in the same way that React.createElement is currently replaced using the jsxFactory setting.

For the Mithral case: Mithral would just need to add a simple function that calls m.fragment(attr, children) if the element is a React.Fragment (whatever this is replaced with ie m.Fragment) and calls the usual m.createElement otherwise - with the user settingjsxFactory to be this simple wrapper function.

[dyst5422]

I think this still belongs to the domain of having the whole JSX type system refer to the factory method type and allow the factory method to deal with empty braces however the particular implementation desires.

[wnayes]

My 2 cents on how this could be implemented...

Proposal

Fragment shorthand syntax <> ... </> indicates a JSX element should be created with a special "Fragment" type. To offer custom JSX factories some flexibility, a new FragmentProperty interface could be added to the JSX namespace that defines what the name of the Fragment type is.

For example, React would define it this way:

declare namespace JSX {
    interface FragmentProperty { Fragment: {}; }
}

If a custom JSX factory does not implement this interface, using the shorthand syntax would be disallowed.

The Fragment type should not be constrained to the JSX.ElementClass, as it's just going to be some constant in most cases, not a class. This is a pain point in React's definitions, where it has to declare the Fragment type as being a ComponentType.

While I generally wish type checking was done through the createElement signature as issue 21699 seems to be covering, the definition of the special Fragment constant seems like a good use of the JSX namespace. The createElement signature can't really indicate which particular type constant is the one associated with the fragment shorthand syntax.

[jhpratt]

@RyanCavanaugh I'm using JSX in a way that it renders native DOM objects, and am currently hitting a roadblock as I'm unable to specify a custom pragma for fragments.

[renchap]

Another use case: Emotion 10 uses a custom JSX pragma to handle the css prop: https://emotion.sh/docs/css-prop#jsx-pragma

[brianmhunt]

Just a data point: This is an issue for TKO/knockout.js as well.

[mmichlin66]

I like the proposal from @wnayes (#20469 (comment)), but I would change one thing: if the FragmentProperty is not specified, then call the custom JSX factory function with either empty string or null or undefined.

[mrgleba]

Any chance this gets some traction?
This would really help to clear and simplyfy our codebase.

[ryansolid]

With Preact X out now with Fragment support that's another library to add to the growing list.

[SteveYurka]

+1 to this

[nojvek]

I am working on adding jsx/tsx support in snabbdom https://github.com//snabbdom/pull/451

Would really love to support jsx fragments too.

I think the proposal of calling jsxFactory(null, null, ...children) is very simple and elegant.

@RyanCavanaugh if you are okay with above proposal that has been mentioned multiple times. I’d be happy to create a PR.

based on developit/vhtml#16

The reasoning being <>Foo</> is kinda essentially an element without a tag name. Most implementations usually just return the children as array which a parent jsxFactory/h call will flatten.

This makes it very simple for jsxFactory: handle null for jsx Factory, a function for a function component and a string for a intrinsic element.

[nojvek]

I've made a fix PR #35392

Copy pasting proposal fix here.

Problem:

Currently <><Foo /></> only works with "jsx": "react". Using an inline pragma for jsxFactory /** @jsx dom */ or config defined "jsxFactory": "h" throws an error that JSX fragment is not supported when using --jsxFactory

The issue has been open for almost 2 years now.

Proposal Fix:

based on developit/vhtml#16

#20469 (comment)

if the FragmentProperty is not specified, then call the custom JSX factory function with either empty string or null or undefined.

I think the proposal of calling jsxFactory(null, null, ...children) is very simple and elegant.

The reasoning being <>Foo</> is kinda essentially an element without a tag name. So null seems intuitive, like the way null is passed if attributes/props aren't defined.

Rather than adding yet another compiler option and pragma, the jsx functions can simply handle null and call their own custom fragment function if needed. Most implementations usually just return the children as array which a parent call stack frame flattens with existing children.

User implementations could do custom things too if they wanted and have full control over how they want to convert tag: null to their version of fragments.

Example:

export type FunctionComponent = (props: { [prop: string]: any }, children?: VNode[]) => VNode;

export function jsx(tag: string | FunctionComponent | null, attrs: VNodeAttrs | null, ...children: VNodeChildren[]): VNode {
  const flatChildren = flattenAndFilterFalsey(children, []);
  if (tag === null) { // fragment
   return MyCustomFragmentFunc(flatChildren);
  }
  else if (typeof tag === "function") { // function component
    return tag(attrs, flatChildren);
  } else {
    return createVNode(tag, attrs, flatChildren);
  }
}

This makes a very simple api for custom jsxFactory functions.

1. a string for an instrinsic element
2. a function for a pure stateless component
3. some instance class defined in JSX namespace for stateful components
4. null for fragments

[nojvek]

update: #35392, supports jsxFrag pragma and jsxFragFactory compiler option.

[fregante]

While TS doesn't yet support custom JSX fragment pragma, you can just make it work with the defaults if your JSX builder is named React.createElement and React.Fragment

https://github.com/vadimdemedes/dom-chef/blob/2dc53d663e640dbdb9bd809d42bdddbfb2c8b8bb/index.js#L104-L109

In this case, we use it as:

import React from 'dom-chef';

export default <>wow!</>;

The drawback is just having a variable named React while you don't use it... which really isn't that big of a deal. JSX was technically born with React.

[sarimarton]

@fregantor This solution promotes unclean code, and is quite smelly. It's also problematic in environments where custom JSX is needed and React API is also used (I know some). JSX being born with React is not relevant, just a historical detail. jsxFactory option doesn't make sense without jsxFragmentFactory

[fregante]

You talk about code smell and then mention code that uses React but then JSX is handled by another library. 🤷‍♂️

I'm just suggesting a workaround at the moment, of course if there's proper solution I'd avoid having React in my code too.

[sarimarton]

Any framework which builds upon react as a rendering engine (don't ask for example), can get in such a situation, nothing smelly in that.

[vbargl]

Any update on this?

Btw. Guys. If I don't know if know about it, but there is a workaround for preact without defining React variable. If you import a Fragment constant and use it as you would do with any other component, then it just works.

import { h, Fragment } from "preact"

export const Component = () => (
    <Fragment>
        // anything in here will not be nested
    </Fragment>
)

[nojvek]

I’ll make another pass on it.

[masx200]

This issue has not been resolved for a long time since the question was asked.
Is there any other plugin to solve this problem?

[sandersn]

@nojvek got a good start on this before having to switch to other tasks. If you are attempting a fix, start from this PR: #35392

[devlato]

Any updates on this? 🙂

[brainkim]

One thought I had just now, what if we transpiled <> elements to an empty string rather than using null? In other words, <>Hello world</> transpiles to createElement("", null, "Hello world"). This might be better in the sense that it keeps createElement monomorphic, and it’s currently impossible to create an element with an empty string for a tag anyways.

I think either null or an empty string would be strictly better because it would mean we reduced the number of config options, and that frameworks themselves can figure out what to do with the syntax for themselves. I also think an empty string is the most natural way to interpret JSX fragment syntax.

[nojvek]

It’s going to be hard to gain support for either null or empty tag. Mostly because the major frameworks all provide a Fragment function like React/Preact. Babel supports jsxFrag pragma.

I guess one can always set “jsxFragmentFactory: ‘’” and get what you’re proposing.

However it makes sense to support the compiler flag to tsc is consistent with Babel and gives user the flexibility to choose rather than forcing its opinion.

[brainkim]

Mostly because the major frameworks all provide a Fragment function like React/Preact.

Most frameworks could probably support factory calls with empty strings meaning fragments very easily, we’d all just have to agree to it.

gives user the flexibility to choose rather than forcing its opinion.

Ehh. We should always be looking for opportunities to lighten the configuration burden we impose on our fellow developers. There is no situation where you have a jsxFactory set to one thing, and then want the jsxFragmentFactory set to something which the framework doesn’t expect. I don’t see this as flexibility but mental overhead, and it gets especially annoying when you use jsx pragmas exclusively.

However it makes sense to support the compiler flag to tsc is consistent with Babel

I agree it would be nice to have consistency with Babel.

[marvinhagemeister]

Mostly because the major frameworks all provide a Fragment function like React/Preact.

Most frameworks could probably support factory calls with empty strings meaning fragments very easily, we’d all just have to agree to it.

I strongly disagree with this. strings are reserved for "host" nodes (most commonly DOM). Every framework that has implemented Fragments as a Component and therefore uses the function type. Deviating from that would cause a huge rift in the ecosystem and would cause a lot of breakage. It would impose an additional complexity tax on every developer having to deal with babel + TS.

There is no situation where you have a jsxFactory set to one thing, and then want the jsxFragmentFactory set to something which the framework doesn’t expect. I don’t see this as flexibility but mental overhead, and it gets especially annoying when you use jsx pragmas exclusively.

I love that you're trying to find ways to simplify cognitive load for fellow developers, but I'd encourage you to look more closely why we're in this "mess" in the first place.

The reason we're in this situation is because facebook has traditionally used their own bundler (I think it's called haste?) and published React only as a CommonJS module. When ES6 started to become a thing many imported React like a default import:

// react
module.exports = {
  createElement() {...}
}

// my-app.js
import React from "react";

const div = <div />

After transpilaton:

// my-app.js
import React from "react";

const div = React.createElement("div", null);

In other words: The createElement function wasn't in scope and had to be pulled of the React object. This lead to both babel and TS to set jsxFactory to React.createElement instead of just createElement. This is the single reason why we need those two flags. Every framework except React doesn't have a React object in scope.

If we'd live in a separate timeline we wouldn't have that problem in the first place. jsxFactory could always be hardcoded to createElement and jsxFragmentFactory to Fragment. As long as those 2 variables are in scope it would work automatically for any framework.

import { createElement, Fragment } from "my-cool-framework";

// const div = <div />;
const div = createElement("div, null);

But we don't live in that world. We have to deal with React.createElement and React.Fragment as the default values. This breaks other frameworks and is the reason we need both flags, and why jsxFactory alone is not sufficent.

[Andarist]

@marvinhagemeister I believe that you have a strong argument about this messy situation being caused by early days' decisions and React being so predominant back then. Some decisions around JSX support would look quite different if implemented for a more general use case, but many have been made with React alone in mind.

It seems to me though that maybe you are in a good position to help to shape the alternative timeline now - by focusing on a good JSX implementation/semantics for the upcoming "auto" mode. IMHO it has the potential to act a clean slate for a couple of things and most likely could solve some of your pain points in one go.

[brainkim]

@marvinhagemeister

strings are reserved for "host" nodes (most commonly DOM). Every framework that has implemented Fragments as a Component and therefore uses the function type. Deviating from that would cause a huge rift in the ecosystem and would cause a lot of breakage.

I blur the lines between host and component elements in Crank a little bit by using Symbols as an alternative to strings. JSX has that tricky limitation where uppercase means identifier and lowercase means string, but if you also make it possible to use symbols, you can import and use uppercase tags which work like host nodes. This makes it easier to support case-sensitive xml in custom renderers. So in Crank (and actually React too), Fragment is a symbol and not a function. I’m also trying to get Typescript to support this pattern here #38367, maybe you have a strong reaction to this too?

Maybe it’s a bad idea, but I also thought there were too many disanalogies to saying a Fragment is a component.

It would impose an additional complexity tax on every developer having to deal with babel + TS.

Correct me if I’m wrong, but couldn’t you just add a check somewhere around here for an empty string and reassign type to the Fragment component?

[brainkim]

Hi. I’m wondering what the reasoning was for explicitly disallowing non-identifiers? I’ve opened an issue (#41400) for the use-case of literals, specifically, the empty string, being used as fragment syntax. A cursory read through the related PR indicates this restriction is mostly artificial but I may be mistaken.