Skip to content
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

Support <> standalone fragment syntax #84

Closed
smrq opened this issue Aug 15, 2017 · 63 comments
Closed

Support <> standalone fragment syntax #84

smrq opened this issue Aug 15, 2017 · 63 comments

Comments

@smrq
Copy link

@smrq smrq commented Aug 15, 2017

This syntax (or similar) has been brought up in a number of comments across various issues already, but as far as I can tell has no standalone proposal.

The syntax is:

<>
  <Child />
  <Child />
  <Child />
</>

This is equivalent to an array, except that because the structure of the array is statically known, keys could be automatically assigned to each child.

I think a lot of people may mistakenly think React 16.x can already do this as a result of the new feature to return multiple components from render. I suspect the lack of this syntax might become even more of a pain point with that release.

(Correct me if I'm wrong or should clarify anything further. I'm just making the issue because it seems there should have been one already.)

@smrq smrq changed the title Support `<>` standalone fragment syntax Support <> standalone fragment syntax Aug 15, 2017
@gaearon
Copy link
Member

@gaearon gaearon commented Aug 15, 2017

cc @SanderSpies, if I recall you have something like this in Reason. How did it work out?

@diligiant
Copy link

@diligiant diligiant commented Aug 15, 2017

@smrq let me reverse position (not the devil's advocate ;) here and say that React 16.x now allows us to do that (by explicitely setting key= in <Child />) ; BUT lots of us would like this to be implicit for the sake of new "users".

@sebmarkbage
Copy link
Member

@sebmarkbage sebmarkbage commented Aug 16, 2017

The key thing is one issue that follows a very specific heuristic. It may not be enough to warrant new syntax. It might be possible to find other ways.

The more convincing argument to me is the ability to nest text content and dropping the comma separator.

return <>
  Hello <strong>World</strong>!
</>

vs.

return [
  "Hello ", <strong>World</strong>, "!"
]

Another variant would be to drop the comma between JSX elements in arrays. This doesn't solve the text content issue though. Maybe that's just an argument for dropping JSXText #35?

return [
  <Foo />
  <Bar />
  <Baz />
]

vs.

return [
  <Foo />,
  <Bar />,
  <Baz />,
]
@sebmarkbage
Copy link
Member

@sebmarkbage sebmarkbage commented Aug 16, 2017

One thing that would be neat is that you sometimes would like to put a key on a whole fragment.

var dynamicList = items.map(item => {
  if (item.type === Foo) {
    return <>
      <Foo />
      <Bar />
    </>;
  }
  return <Foo key={item.id} />;
});

In the proposed <> syntax there is no syntactic space to define a key. If you put something in there such as "frag" then there is.

var dynamicList = items.map(item => {
  if (item.type === Foo) {
    return <Frag key={item.id}>
      <Foo />
      <Bar />
    </Frag>;
  }
  return <Foo key={item.id} />;
});

This can be implemented today in React as a component.

@isiahmeadows
Copy link
Contributor

@isiahmeadows isiahmeadows commented Aug 16, 2017

I'll also point out that non-React usage of this would also greatly benefit (like Mithril + @jsx m, where Mithril already supports fragments).

@wmertens
Copy link

@wmertens wmertens commented Aug 16, 2017

The issue with Frag is that it is in the same namespace as regular components.

How about <[]>…</[]>? => <[] key="foo">…</[]>

Or this might work: <{...{key: 'foo'}}>…</>

@ljharb
Copy link

@ljharb ljharb commented Aug 16, 2017

What about something like <fragment />, which would be a host component that React could define?

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

Another proposal was to use namespaces.

<react:fragment>
  <Child />
  <Child />
</react:fragment>

But if we were to do this, should we go all-in? For example key and ref are also React-specific (and technically don’t become props) so we could also do

<div react:key="1" react:ref={fn} />

But this is awkward to assign when defining element config as plain object.

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

What about something like <fragment />, which would be a host component that React could define?

This blocks HTML from ever defining it. Kind of like MooTools killed [].contains().

@ljharb
Copy link

@ljharb ljharb commented Aug 16, 2017

document.createDocumentFragment() already exists and HTML is highly unlikely to ever define <fragment>, but fair point 😬

I like the namespacing idea.

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

A few more ideas.

<@fragment>
  <Child />
  <Child />
</@fragment>
<$>
  <Child />
  <Child />
</$>
@wmertens
Copy link

@wmertens wmertens commented Aug 16, 2017

Or, use <> and when you do need to add a key, do-it-yourself:

const Frag = ({children}) => children

React could provide this as a named export of course.

I really like how obvious the <> syntax is. OTOH. It prevents<> being defined as a JS operator at some point.

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

I like this.

<*>
  <Child />
  <Child />
</*>

It kind of implies "multiple elements" and you can put a key on it if you want.

var dynamicList = items.map(item => {
  if (item.type === Foo) {
    return (
      <* key={item.id}>
        <Foo />
        <Bar />
      </*>
    );
  }
  return <Foo key={item.id} />;
});
@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

Haha I guess that won't work because of comments 😛

@ljharb
Copy link

@ljharb ljharb commented Aug 16, 2017

you could always end it with </> :-p

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

The reason I dislike typing "fragment" is because it's one of its kind. It's not like we're going to add more primitives like this. It's pretty fundamental by itself, rather than a part of some bigger API of core primitives. So it's annoying to squat on a whole namespace or create a special export just for a single thing. If you see react:fragment you might expect there to also be react:if, react:for etc.

There's two counter arguments. One is making it explicit makes it very easy to Google. Googling characters is not very pleasant (although maybe they fixed it).

The other counter argument is we may in fact add more primitives. Such as yields and coroutines. But those seem to be solved better on syntax level than by adding special cases of elements. Especially since they're not even producing elements.

@syranide
Copy link
Contributor

@syranide syranide commented Aug 16, 2017

I mean you could support <key="..."></> if you wanted to. Although it may be somewhat confusing. (< key="..."></> is also possible but looks rather weird in context.)

<[arbitrary char symbol]> (e.g. <!>) seems like the obvious go-to but it tends to look just like a jumble of symbols (especially the closing tag, </!>).

<[arbitrary char symbol]fragment> (e.g. <!frag>) is nice in that it's descriptive and not really special, but it also becomes rather wordy. For fragments which is meant to signify something "less" than a regular element, it certainly looks like a lot more.

Personally. Despite its strangeness, I kind of like <> and <key="">, it looks like a fragment (i.e. nothing, just a hollow container) and it's short so that you can reasonably have it on a single line in some cases.

return <key="foobar">
    <div />
</>;
return (
  <key="foobar">
    <div />
    <key="barfoo"> // someone could do this, but I'm not sure why they would
      <strong />
    </>
    <FooBar />
  </>
);

Or is it too strange to stomach?

@rikkit
Copy link

@rikkit rikkit commented Aug 16, 2017

Can someone explain why this wouldn't work?

return (
  <div>Element 1</div>
  <div>Element 2</div>
);

Which could just be sugar for

return [
  <div key="key1">Element 1</div>,
  <div key="key2">Element 2</div>
];

Afaik it doesn't conflict with anything + doesn't need new syntax.

@syranide
Copy link
Contributor

@syranide syranide commented Aug 16, 2017

@rikkit Even if that would work it wouldn't solve <frag>You have <Counter /> items</frag>.

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 16, 2017

Here's an example of ambiguity this would introduce.

return (
  <div>Element 1</div>
  ) this is just a paren in text node
  <div>Element 2</div>
);

Note how text nodes (which are normally valid in JSX) become broken as a result. It's not clear if ) is text or intends to close the opening paren.

There are similar issues with newlines, braces, etc. All of those are OK in JSX but become ambiguous if we don't have a shared JSX ancestor.

@mnpenner
Copy link

@mnpenner mnpenner commented Aug 16, 2017

I'd just like to point out that .net/razor templating language introduces <text> for defining bare text blocks (no wrapping element), so it's not unheard of to introduce special tags (within the "HTML namespace").

I think I'd prefer either <frag> or <> but I'm not sure about this <key="x">...</> idea. It looks too unbalanced to me; even as I wrote that I tried typing </key>. I think IDEs will erroneously try to autocomplete that closing tag too actually.

A user-space <Frag> really isn't so bad except I imagine a great deal of libraries would be re-implementing the same thing over and over again, so it's probably better to make it core.

@syranide
Copy link
Contributor

@syranide syranide commented Aug 16, 2017

@mnpenner This is the JSX-namespace though, and not the HTML-namespace. Also, by reserving the frag element you can no longer create <frag /> elements in HTML, which is legal.

EDIT: "JSX-namespace" as in it applies to all current and future JSX/React renderers, not just HTML.

I think IDEs will erroneously try to autocomplete that closing tag too actually.

Unless JSX ends up reserving an element name then the IDEs need to update the autocomplete and syntax highlighter anyway.

@mnpenner
Copy link

@mnpenner mnpenner commented Aug 16, 2017

@syranide

This is the JSX-namespace though, and not the HTML-namespace.

What do you mean? <a>, <span>, <div> and all the other lowercase tags represent HTML elements. <frag> would fall into that space.

Oh... you mean for people that are using JSX for non-ReactDOM? Yeah, okay. I see your point. Using <frag> would use up a token.

Unless JSX ends up reserving an element name then the IDEs need to update the autocomplete and syntax highlighter anyway.

<frag>, <Frag>, and <react:frag> [I guess we should call it <jsx:frag> then, no?] will all autocomplete today. <> OTOH, does indeed confuse PhpStorm and probably most other editors. Not that it's a huge deal -- I'm sure they'll be updated.

Anyway, I'm not sure what the best solution is then.

@isiahmeadows
Copy link
Contributor

@isiahmeadows isiahmeadows commented Aug 17, 2017

BTW, I like the namespace best:

  • React's fragments are just plain fragments with children. No attributes are supported.
  • Mithril's fragments can potentially have attributes, and this is officially supported.
  • Mercury's virtual-dom backend doesn't support fragments at all.
  • nativejsx (JSX to straight DOM) doesn't even really need fragments.

Here's how I feel it could be handled, via modifications to babel-plugin-transform-react-jsx:

  1. Add fragName: string - the name used to define fragments. This could either be a JSXIdentifier, JSXMemberExpression, or JSXNamespace. This defaults to whatever React ends up using.
  2. Add fragAttrs: boolean - whether to prepend the attributes to the call. This defaults to false, as it's a limited use case.
  3. Add fragPragma: string - the function to replace fragments with, like the existing pragma option or inline @jsx foo. This defaults to whatever React ends up using.
  4. If a JSXElement has an opening element whose tag matches fragName, it's transformed to a call to the function described by fragPragma. If fragAttrs is true, the attributes (or null if none are passed) are the first argument. The children are appended as the remaining arguments.
@gajus
Copy link

@gajus gajus commented Aug 20, 2017

Is this syntax necessary?

Just leave it up to the community to establish a convention. See:

@syranide
Copy link
Contributor

@syranide syranide commented Aug 20, 2017

@gajus Its semantics differ from "true fragments" and has additional performance overhead.

@gajus
Copy link

@gajus gajus commented Aug 21, 2017

@gajus Its semantics differ from "true fragments" and has additional performance overhead.

Sorry, rushed into discussion. Regardless, I disagree that a new syntax (such as <> or <*>, anything thats not already valid JSX) is necessary.

Would it not be better to reserve a component name for the use case? (e.g. Aux, Fragment or whatever else) and treat it specifically.

Otherwise, every new edge case requirement such as this will require a new syntax. Syntax such as <> makes the code uneasy to the eye (because it breaks the JSX flow), it will require updating bunch of linters/ formatters/ plugins that depend on JSX syntax, and it is hard to Google/ describe unambiguously for the purpose of reporting issues/ documenting behaviour/ communicating in person the subject.

@rattrayalex
Copy link

@rattrayalex rattrayalex commented Oct 7, 2017

Yes, a namespace is one way of not stepping on anyone's toes; I'd hope that a shortening like <frag> would accomplish the same goal without as much ugly, but <react:fragment> certainly is unambiguous, so I wouldn't shy away from it. (It might seem like a "scary lib internal" to users though)

@isiahmeadows
Copy link
Contributor

@isiahmeadows isiahmeadows commented Oct 7, 2017

@rattrayalex To clarify, the namespace usage would be optional with my proposal.

@syranide
Copy link
Contributor

@syranide syranide commented Oct 8, 2017

@rattrayalex For React it seems it will be synonymous with <React.Fragment> (or importing Fragment into the local scope).

@gajus
Copy link

@gajus gajus commented Oct 8, 2017

@rattrayalex For React it seems it will be synonymous with <React.Fragment> (or importing Fragment into the local scope).

Which sounds reasonable. Then just have a babel rule that interprets the matching component in a special way.

@jixunmoe
Copy link

@jixunmoe jixunmoe commented Oct 14, 2017

Workaround that works with React v16.0.0:

import React from 'react';
import { render } from 'react-dom';

class VirtualDiv extends React.Component {
  render() {
    return this.props.children;
  }
}

const App = () => (
  <VirtualDiv>
    <h2>Hello</h2>
    <p>Second element</p>
  </VirtualDiv>
);

render(<App />, document.getElementById('root'));

Preview: https://codesandbox.io/s/0m7204mlw

Oops, there's Frag somewhere in the middle of this post that does the same thing I missed. Sorry for the duplicated post.

@wmertens
Copy link

@wmertens wmertens commented Oct 14, 2017

@JonDum
Copy link

@JonDum JonDum commented Oct 16, 2017

Excuse my ignorance on the subject, but instead of introducing new syntax, could we not make JSX introduce a transparent wrapper around static arrays that allows consuming libraries to know for sure that they are static? Then the warnings for keys can be discarded with no new syntax, no performance implications—everyone is happy, world peace is attained, etc.

Example:

function render() {
  return [
     <div>One</div>,
     <div>Two</div>,
     <div>I am a snowflake!</div>
  ]
}

Would result in:

function render() {
   return React.StaticArray(
      React.createElement( "div", null, "One"), 
      React.createElement( "div", null, "Two"), 
      React.createElement( "div", null, "I am a snowflake!")
   );
}

Where React.StaticArray is a simple subclass of Array

export class StaticArray extends Array {
    constructor(...args) { 
        super(...args); 
    }
    __static__ = true
}
@syranide
Copy link
Contributor

@syranide syranide commented Oct 16, 2017

@JonDum Fragments are not really about the data representation, it's to have a useful syntax for this problem: #84 (comment)

@clemmy
Copy link
Contributor

@clemmy clemmy commented Oct 16, 2017

@JonDum In your example, how would the transpiler know that it should be a StaticArray?

@dantman
Copy link

@dantman dantman commented Oct 16, 2017

Excuse my ignorance on the subject, but instead of introducing new syntax, could we not make JSX introduce a transparent wrapper around static arrays that allows consuming libraries to know for sure that they are static? Then the warnings for keys can be discarded with no new syntax, no performance implications—everyone is happy, world peace is attained, etc.

This is a static fragment that is passed to a function that expects some type of element and wraps it.

render() {
	const wrapWithContainer = (content) => <div>{content}</div>;

	return (
		<div>
			{wrapWithContainer([
				<span>A</span>,
				<span>B</span>
			])}
		</div>
	);
}

This is an array of nodes passed to a function that expects an array and does array operations on it.

render() {
	const processNodes = (nodes) => nodes.map((node) => <div>{node}</div>);

	return (
		<div>
			{processNodes([
				<span>A</span>,
				<span>B</span>
			])}
		</div>
	);
}

How do you tell the difference?

@JonDum
Copy link

@JonDum JonDum commented Oct 17, 2017

how would the transpiler know that it should be a StaticArray?
How do you tell the difference?

@clemmy @dantman

You can use the AST to check for the few scenarios where it could be processed, like ensure we're in the render function, we're not a child of CallExpression (i.e., processNodes([])) or we're in a JSXExpressionContainer (not shown in example)

Example:

module.exports = function(babel) {
    const { types: t } = babel;

    const StaticArray = (args) => {
        return t.newExpression(t.Identifier("StaticArray"), args)
    }

    return {
        visitor: {
            ArrayExpression(path) {
                let fp = path.getFunctionParent();

                const isInRenderFunc = (fp.node.id.name == 'render');
                const isReturnedDirectly = path.parent.type == 'ReturnStatement';
                const isNotTransformed = path.parent.type !== "CallExpression";

                if (isInRenderFunc && (isReturnedDirectly || isNotTransformed)) {
                    path.replaceWith(StaticArray(path.node.elements));
                }
            }
        }
    };
}

Quick n' dirty tests:

https://github.com/JonDum/jsx-static-array-plugin

I'm sure I'm missing cases, but the concept is there. Maybe at this point though it's cleaner to just auto-key the elements instead of wrapping in a special Array subclass.

@matt-landers
Copy link

@matt-landers matt-landers commented Oct 30, 2017

I created a component to handle this that I called a shadow wrapper. I wrap my child components in it instead of DIVs. I'd be interested to know it's limitations. I'm fairly new to React.

https://www.npmjs.com/package/react-shadow-wrapper

@styfle
Copy link

@styfle styfle commented Oct 30, 2017

@clemmy @sebmarkbage @smrq Should this issue be closed now that #93 and #94 are merged in?

@clemmy
Copy link
Contributor

@clemmy clemmy commented Oct 30, 2017

@matt-landers That's a good question - the difference is in the state preservation behavior when using a class or functional component to wrap it. Although the shadow wrapper works for common cases, it doesn't preserve state in some edge cases such as rendering something like

ReactDOM.render('<Stateful key="a" />')

and then having it be inside a fragment in a subsequent render

ReactDOM.render('<><Stateful key="a" /></>')

Cases can be found here.

Aside from that, there are some optimizations that can be done by putting the React Fragment export inside the core library.

@matt-landers
Copy link

@matt-landers matt-landers commented Oct 30, 2017

@clemmy Thanks for the response. Looking forward to using the new spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.