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

Better JSX #23

Closed
rauchg opened this issue Oct 16, 2016 · 24 comments
Closed

Better JSX #23

rauchg opened this issue Oct 16, 2016 · 24 comments

Comments

@rauchg
Copy link
Member

rauchg commented Oct 16, 2016

JSX as used with most React projects comes with a very obscure situation
The following doesn't work:

export default () => <div />

You need to import React from 'react' so that the JSX transformation to React.createElement, which appears nowhere in the source, works.

How do we fix this?

@rauchg
Copy link
Member Author

rauchg commented Oct 25, 2016

We could also call this issue "Universal JSX"

@tolmasky
Copy link

Not sure if you've taken a look at the work I've done with generic jsx (http://tolmasky.com/2016/03/24/generalizing-jsx/), but it is a re-imaging of jsx as a pure syntax transformation (that then could incidentally be used to represent inline HTML), which could serve these goals. Essentially, any function can be used as a JSX tag, and it has some nice benefits like being able to curry tags as well (OurTable = <Table style = { style }/>; <OurTable>...</OurTable>). We've successfully used it in DemoKit, but it would take a bit of work to make work with react, although it aircraft includes a babel plugin.

@rauchg
Copy link
Member Author

rauchg commented Oct 25, 2016

I think generalizing it is absolutely critical. Thanks for the pointer.
Otherwise we'll be perpetually in the limbo of "it's JavaScript buuuuut not
quite".

On Tuesday, October 25, 2016, Francisco Ryan Tolmasky I <
notifications@github.com> wrote:

Not sure if you've taken a look at the work I've done with generic jsx (
http://tolmasky.com/2016/03/24/generalizing-jsx/), but it is a re-imaging
of jsx as a pure syntax transformation (that then could incidentally be
used to represent inline HTML), which could serve these goals. Essentially,
any function can be used as a JSX tag, and it has some nice benefits like
being able to curry tags as well (OurTable = <Table style = { style }/>;
...). We've successfully used it in DemoKit, but it
would take a bit of work to make work with react, although it aircraft
includes a babel plugin.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#23 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAy8UujGYS8GIHnqPciJS-JyW2ddqcMks5q3jC1gaJpZM4KYHmr
.

@rauchg https://twitter.com/rauchg

@trueadm
Copy link

trueadm commented Oct 26, 2016

I don't think it's ideal to to expect all objects to be inline. You can get big performance wins on hoisting static elements out, which would fail in this case as they'd be hoisted outside of the exported function. Inferno takes this further and uses hoisting all over the place to get really good performance/memory wins.

This is a tricky problem, I'll have a think on how to solve it.

@matthewmueller
Copy link
Contributor

matthewmueller commented Oct 26, 2016

This has been working quite well for me: https://github.com/matthewmueller/sun. Right now it only works with Preact, but could work in React with some cajoling.

Works in ES6, ES5, ES3... it's just functions.

@trueadm
Copy link

trueadm commented Oct 26, 2016

@matthewmueller I don't see how that addresses the issues here? React elements can be hoisted, as can Preact ones – for a good reason, they're very performant if they do. What you're suggesting actually reduces performance as this optimisation isn't able to apply itself.

@trueadm
Copy link

trueadm commented Oct 26, 2016

For example: if I did the following in my code:

function MyComponent() {
  return <div className="something"><span>Hello world</span></div>
}

With hoisting it becomes:

var hoisted = <div className="something"><span>Hello world</span></div>;

function MyComponent() {
  return hoisted
}

The performance difference according to my benchmarks to mount and update is 650%. Now hopefully you understand the importance of this feature.

@matthewmueller
Copy link
Contributor

matthewmueller commented Oct 27, 2016

@trueadm oh okay, up until your first comment this sounded more like a developer experience issue (needing a magic import react + forgetting the slash on <div/>).

thanks for the insight there. while i haven't seen VDOM tree rendering be a performance bottleneck in any of my apps (even with top to bottom re-renders every time), it's good to keep in mind.

@rauchg
Copy link
Member Author

rauchg commented Oct 27, 2016

@trueadm I think it should still be possible to perform that optimization even if we go with a generic jsx transform?

@developit
Copy link
Contributor

developit commented Oct 27, 2016

FWIW, this only affects static leaves. Any JSX containing variable references cannot be hoisted. In my experience this is the majority of JSX, but it varies depending on your use-case.

Also, functions can be memoized to achieve similar performance gains without relying on any transpiler features.

@trueadm
Copy link

trueadm commented Oct 27, 2016

@developit Unfortunately, Inferno doesn't work this way :/ it creates blueprints that are hoisted out and referenced in the vDOM.

@developit
Copy link
Contributor

True, though that seems outside the scope of JSX itself.

@developit
Copy link
Contributor

@rauchg we could avoid the extra function invocation by making Next.createElement just a reference to the appropriate underlying hyperscript implementation rather than a proxy function.

@trueadm
Copy link

trueadm commented Oct 27, 2016

@developit in fact after thinking about this more, I don't see why this is an issue. Can't Next simply look at the file AFTER Babel has compiled JSX and then use that code?

@developit
Copy link
Contributor

@trueadm You mean transpile to something like Next.createElement but then rewrite those to point to whatever implementation is in use?

@trueadm
Copy link

trueadm commented Oct 27, 2016

@developit No need. If someone was using JSX in their module, they would have put import React from 'react'; at the top of their module, so the whole module should get passed to the client in that case, not just the function.

@developit
Copy link
Contributor

I might be mixing things up here - I believe @rauchg was advocating for removing the import entirely and automatically injecting the appropriate JSX reviver based on the configured renderer.

@trueadm
Copy link

trueadm commented Oct 27, 2016

@developit Why go to the hassle of that? It's only going to bring issues downstream when more renderers are added. I'd much rather the configured renderer also could define its own Babel transforms. The "default" renderer, aka React would define the React Babel transforms, etc.

@developit
Copy link
Contributor

ahh yeah that seems simpler, since the babel config is created at a time when the renderer would already have been specified.

@SeeThruHead
Copy link

SeeThruHead commented Oct 28, 2016

I encountered something like this in a project recently that had to be built for both react and preact.
Solved the issue with different babel env's and a babel plugin called jsx-pragmatic

@developit
Copy link
Contributor

@SeeThruHead seems like that plugin could actually do basically all of the work for this issue haha

@rauchg
Copy link
Member Author

rauchg commented Nov 12, 2016

@trueadm I really like the idea of the renderer defining transforms!

@rauchg
Copy link
Member Author

rauchg commented Nov 12, 2016

Additionally, we can have the renderer extend the config any way it wants:

decorate(config)

function decorateForInferno (cfg) {
  return Object.extend({}, cfg, {
    cfg.cdn: false, // or your own build of the next cdn
    cfg.jsxPragma: 'Preact.createElement'
  })
}

If we switch to practical JSX we can make the pragma a config option

@rauchg
Copy link
Member Author

rauchg commented Dec 6, 2016

I'm closing this for the time being. We addressed the import problem with #295. We're addressing custom renderers by giving them the ability to change webpack and babel configs :)

@rauchg rauchg closed this as completed Dec 6, 2016
@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants