babel / babel

🐠 Babel is a compiler for writing next generation JavaScript.
https://babel.dev
MIT License
43.16k stars 5.63k forks source link

Use Array for JSX Children #2034

Closed killercup closed 9 years ago

killercup commented 9 years ago

Thanks to the jsxPragma option, it should be possible to use Babel's JSX transform with virtual-dom's helper module virtual-hyperscript. But because of one tiny detail, it doesn't work for me.

virtual-hyperscript supports the signature h(selector: String, properties: Object, children: Array). Sadly, Babel appends the children as separate arguments, not as array.

E.g., this:

function stuff() {
  return <div>
    <p>test</p>
    <p>test</p>
    <p>test</p>
    <p>test</p>
  </div>;
}

gets transformed into this (plus boilerplate):

function stuff() {
  return h("div", null,
    h("p", null, "test"),
    h("p", null, "test"),
    h("p", null, "test"),
    h("p", null, "test")
  );
}

(This example in Babel's online REPL.)

The output needed for virtual-dom would be this tiny variation:

function stuff() {
  return h("div", null, [
    h("p", null, "test"),
    h("p", null, "test"),
    h("p", null, "test"),
    h("p", null, "test")
  ]);
}

Since the array syntax also works for React.createElement, this change would not break any code.

The virtual-dom helper allows supplying a single element instead of an array if there is only one, so a special case for children.length === 1 with no array allocation would be possible. (I'm not sure what the performance characteristics of using arguments are nowadays; a few years ago the overhead of calling functions with a variable number of arguments prevented some optimizations, IIRC.)

sebmck commented 9 years ago

I'm not interested in supporting alternate JSX output signatures in the core react JSX transformer, you can alternatively write your own plugin that transforms the JSX to your desired output.

The performance characteristics of using arguments over an array allocation would be far better as the array allocation is unnecessary when you can have variable length arguments which (AFAIK) have no negative performance benefits on modern JITs.

Thanks! :blush:

killercup commented 9 years ago

Thanks for the quick answer, @sebmck! I'll investigate what's more complicated then, writing that plugin or changing virtual-hyperscript :)

killercup commented 9 years ago

FYI, I'm using jsx-transform for now. Not the most efficient solution, but no work at all for me :)

(Adding variable length arguments to virtual-hyperscript was discussed and rejected in Matt-Esch/virtual-dom#138.)

jmm commented 9 years ago

@killercup Don't know if it would preferable to your current solution, but you could alternatively do something like this:

jsx-create-element.js

import h from 'virtual-dom/h';

export default function (el, props, ...children) {
  return h(el, props, children);
};
killercup commented 9 years ago

@jmm I'm using Cycle for one project and it added just that a few days ago: https://github.com/cyclejs/cycle-dom/commit/e88c8e5472a573a7d7c38bad903170035e710d32

jmm commented 9 years ago

@killercup Oh, interesting.

ghost commented 9 years ago

The tight coupling to react in a core plugin that is automatically enabled at the expense of other implementations is worrisome.

sebmck commented 9 years ago

@substack The transformer is actually called react so it's not as inappropriate as it seems.

ghost commented 9 years ago

But the plugin is enabled by default, making these features extremely opaque to the point where they make up an integral part of the main functionality. Disabling or modifying a feature you never enabled in the first place is always harder than changing a feature that you've taken affirmative steps to enable.

sebmck commented 9 years ago

Completely agree which is why the next major version of Babel is going to have no transformers enabled by default. There will be presets available (as modules on npm) that you can use that are collections of plugins to ease the pain of assembling your own config. For example a React users .babelrc file may look like:

{
  "presets": ["react", "es2015"],
  // other babel options here
}

Where each name in presets would refer to a package like babel-preset-react that would consist of an index.js of something like:

module.exports = {
  plugins: [
    // enable flow and jsx syntax in the parser
    require("babel-plugin-jsx"),
    require("babel-plugin-flow"),

    // convert jsx nodes to react ones
    require("babel-plugin-react-jsx"),

    // convert flow nodes to comments
    require("babel-plugin-flow-comments"),
  ]
};

This is natural progression in making Babel a more standard compiler for JavaScript.

ghost commented 9 years ago

This sounds like a good direction for babel to take. Thanks for responding!

ghost commented 9 years ago

For internet travelers who find this thread in the future: babel-plugin-jsx-factory implements the requested signature of h(tagName, attributes, children)

jsg2021 commented 9 years ago

Just to comment on the original post:

Since the array syntax also works for React.createElement, this change would not break any code.

@killercup that's an invalid statement.

React.createElement's signature is (type, props, ...children) where each child is supposed to be a first level argument, not wrapped into an array. If you wrap the children into an array, you're actually creating a hidden intermediate child with children, and _react will warn about missing key props._

sebmck commented 9 years ago

@substack Opened issue #2168 to discuss 6.0 and the transformless Babel that I talked about a bit. Would appreciate your feedback if you have any!

jabez128 commented 8 years ago

@jmm nice solution~

babel-bot commented 8 years ago

Comment originally made by @kuraga on 2015-12-22T06:41:38.000Z

! In T2034#56035, @sebmck wrote: The performance characteristics of using arguments over an array allocation would be far better as the array allocation is unnecessary when you can have variable length arguments which (AFAIK) have no negative performance benefits on modern JITs.

@sebmck , seems like it's not correct, please see benchmark: http://jsperf.com/function-signatures

Give a reaction, please! Thanks!

ngryman commented 7 years ago

I'm reviving this issue because passing children as arguments does have a real negative performance impact. It would make lots of library authors lives easier to just have to handle a clean function signature.

Could we have an optin for this like jsx-transform?

Thanks!


ref: https://github.com/hyperapp/hyperapp/issues/183

jsg2021 commented 7 years ago

If you pass children as an array react will warn about keys.

loganfsmyth commented 7 years ago

I don't think anyone would object to an option to enable this behavior. It's unlikely to be a priority for us to implement since we're focusing on more architectural stuff for 7.x, but if someone wants to put it together I'd be happy to review.

marcelklehr commented 7 years ago

This is a problem for me, too. Especially, since all the other ways of doing this seem to not work (anymore?).