-
Notifications
You must be signed in to change notification settings - Fork 28.2k
Better JSX #23
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
Comments
We could also call this issue "Universal JSX" |
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 ( |
I think generalizing it is absolutely critical. Thanks for the pointer. On Tuesday, October 25, 2016, Francisco Ryan Tolmasky I <
|
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. |
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. |
@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. |
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. |
@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 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. |
@trueadm I think it should still be possible to perform that optimization even if we go with a generic jsx transform? |
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. |
@developit Unfortunately, Inferno doesn't work this way :/ it creates blueprints that are hoisted out and referenced in the vDOM. |
True, though that seems outside the scope of JSX itself. |
@rauchg we could avoid the extra function invocation by making |
@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? |
@trueadm You mean transpile to something like |
@developit No need. If someone was using JSX in their module, they would have put |
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. |
@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. |
ahh yeah that seems simpler, since the babel config is created at a time when the renderer would already have been specified. |
I encountered something like this in a project recently that had to be built for both react and preact. |
@SeeThruHead seems like that plugin could actually do basically all of the work for this issue haha |
@trueadm I really like the idea of the renderer defining transforms! |
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 |
I'm closing this for the time being. We addressed the |
JSX
as used with mostReact
projects comes with a very obscure situationThe following doesn't work:
You need to
import React from 'react'
so that the JSX transformation toReact.createElement
, which appears nowhere in the source, works.How do we fix this?
Next.createElement
which invokes the right adapter (Support for pluggable renderers #20). Downside: extra invocationThe text was updated successfully, but these errors were encountered: