Rendering fragments
darsain opened this issue · 47 comments
Or in other words, support for components to render multiple children:
const Component = () => [<Child1 />, <Child2 />];It was said on slack it's a wanted feature with intention to be implement in future, so I'm creating this issue to have a place to track the progress.
Someone mentioned that it was already once in Inferno, but at the cost of performance, so it was dropped until a faster implementation is discovered. Was the performance hit really that bad? Because I'd gladly take it if it means the ability to render fragments. Inferno is one of the fastest frameworks out there, so it definitely doesn't lack the perf budget.
I consider this to be an essential feature to not have DOM polluted with useless wrapper elements which just get in the way of developing, maintaining, and styling components. And React already supports this in their Fiber rewrite, which is mostly done.
Mounting fragments is cheap, it's the moving/removing fragments that introduces complications since they can be seen as essentially a paired set. I'm interested in knowing how React-Fiber rewrite does this if they do and to what extent.
having a similar convo over at domvm/domvm#106
@thysultan support for fragment rendering in React Fiber is mentioned here:
https://github.com/acdlite/react-fiber-architecture#child-and-sibling
Also I've heard it described in one talk, but can't recall enough about it to find it again :)
And if we are comparing different solutions, mithril (rewrite branch) also supports it.
Mithril also flattens all children by default (pretty useful when templating in hyperscript), so you can pass nested arrays and it gets resolved properly. Dunno if that is also the behavior in Inferno/React.
So this will likely going in for the future along with my refactor of normalization within Inferno. It won't be in for 1.0 due to the amount of edge cases it brings and things it breaks for now.
#628 is now closed. Should this issue be reopened now that React 16 is out and has this feature?
Yeah lets re-open, it will be there in next major release.
This will come in v5
@Havunen I see you've been doing some work on this recently - does it look like it may be released soon?
Next big release will be v4, which does not include fragments. They were dropped out due to some bugs. Also fragments are in jsx plugin which is currently in beta. I think its better to wait babel / facebook to stabilize the feature and then we can implement it.
v5 is out, is this now supported?
@SeanHayes V5 was in this case a major breaking change fix with esm modules. So this will most likely come with V6
Unfortunately Next.js is using the short syntax of React.Fragment for it's Container's render method, which breaks usability of next.js with Inferno 😞
Related issue at next.js:
vercel/next.js#4031
Here is the original feature discussion for fragments in jsx:
facebook/jsx#84
This could also be solved by babel JSX plugin similar to:
https://github.com/jquense/babel-plugin-jsx-fragment
We need Fragment, more and more React components using it.
So what happened? is there any progress about this feature? I'd love to know :-)
I`m in USA FL atm. This is currently first task in my todo list. I will continue it Aug 6th unless somebody wants to send PR :-)
This is now finally out for testing! Please see
https://github.com/infernojs/inferno/releases/tag/v6.0.0-rc.0
Hmm i tried the last one v6.0.0-rc.1 still Fragment is not working for me
@simonjoom do you use JSX? Did you remember to update babel-plugin-inferno to 6.0.0-5https://github.com/infernojs/babel-plugin-inferno/blob/dev/package.json
Are you getting any error message?
i ve got the latest inferno took from github and builded manually.
But yes i think i should to try with the babel you mention (i used only web pack aliases)
I created a repo merging gatsby with inferno, it's working nice)
hello, i just tried without success with the branch dev as well.
Fragment is not working.
the plugin is well installed with webpack and babel config
for a tag <>blabla</>
i ve got this error
Inferno Error: createElement() name parameter cannot be undefined, null, false or true, It must be a string, class
So i tried with
import {Fragment} from 'React
<Fragment >blabla</Fragment>
I ve got "export 'Fragment' was not found in 'react' as well
i use ssr of gastby not sure it's related but..
Thanks
@simonjoom I believe you're meant to use Inferno.createFragment() if you're not intending to use the Babel transform (or maybe you're not using the latest version?). The base API is not a 1:1 of the React API, it's a different API that the Babel transform makes look similar.
Hmm, did you configure the babel plugin correctly @simonjoom?
Check out if the fragment syntax is correct (reference here https://github.com/infernojs/babel-plugin-inferno/blob/6071ba7c24106ad4eb57f1d7ffbf8e8271f9b456/README.md#fragments)
And check if your configuration in .babelrc is correct (reference here https://github.com/infernojs/babel-plugin-inferno/blob/6071ba7c24106ad4eb57f1d7ffbf8e8271f9b456/README.md#how-to-use)
The blobs point to the latest release of that plugin
6.0.0-5
I can't find the babel plugin in your gatsby starter. If you do not intent to use it, you must write the fragments using what @trueadm suggested (check out the tests for the plugin's fragments implementation - basically what the plugin transforms your jsx code into infernojs/babel-plugin-inferno@6071ba7#diff-ce4097fa41097a3b63603226458faf55R309)
I think the issue was because new methods were not exported from inferno-compat. I will make new release
@simonjoom can you test inferno-6.0.0-rc.3 this was probably issue with Inferno compat pkg
hello yes it's working great now:) thanks
just i saw a little bug if a fragment is around a single react node,
ok i know it's stupid to do
<><myComponent/></>
but it should to render <myComponent/> and not return null.
Maybe ping KyleAMathews from gatsby about the status of inferno, i think it's interesting for gatsby now.
look my thread on it:
gatsbyjs/gatsby#8858
Hmm, did you configure the babel plugin correctly @simonjoom?
Check out if the fragment syntax is correct (reference here https://github.com/infernojs/babel-plugin-inferno/blob/6071ba7c24106ad4eb57f1d7ffbf8e8271f9b456/README.md#fragments)And check if your configuration in
.babelrcis correct (reference here https://github.com/infernojs/babel-plugin-inferno/blob/6071ba7c24106ad4eb57f1d7ffbf8e8271f9b456/README.md#how-to-use)The blobs point to the latest release of that plugin
6.0.0-5I can't find the babel plugin in your gatsby starter. If you do not intent to use it, you must write the fragments using what @trueadm suggested (check out the tests for the plugin's fragments implementation - basically what the plugin transforms your jsx code into infernojs/babel-plugin-inferno@6071ba7#diff-ce4097fa41097a3b63603226458faf55R309)
about the starter repo i did, i did not updated properly it, so well yes babel-plugin was not well configured when you wrote these lines.
I just did now on this commit
simonjoom/gatsby-starter-inferno-master@c03916b
@simonjoom if you are rendering component it needs to be written <><MyComponent/></> notice uppercase, otherwise its handled as vNode element ( div, span etc... ).
That use case should work unless there is bug somewhere.
For now; Please check test cases for documentation: https://github.com/infernojs/inferno/blob/master/packages/inferno/__tests__/fragments.spec.jsx
yes sorry i wrote badly here. but on my app it's well written like:
<><MyComponent/></>
and it's return nothing in rendering so yes it's a bug from inferno
Nota if i add an other component or a node inside then MyComponent well render
<><div/><MyComponent/></>
@simonjoom Interesting, can you create steps to reproduce it somewhere? Maybe github repository? I need to be away for a while, but when I'm back I would like to have a look at that issue.
@simonjoom I Just published 6.0.0-rc.5 and babel-plugin 6.0.0-6 It fixes issue with empty fragments and when hydrating Fragments. Can you test if that resolved your issue. Thanks for all the help!
ok do that now :)
i have got a big project so i could find some bug for sure ))
hello ok the issue of single fragment is still here unfortunately with the last version you asked me to try.
Nota: now it's returning [{object}, {object}] in the html not anymore null (i'm not sure it's can help...)
Anyway looking at the source code i found that in develop mode i ve got some:
<div class="app" __source="[object Object]" __self="[object Object]">
was the same thing than preactjs/preact#1058
I added pragma and this thing was resolved for me too ))
Because i use pragma I can confirm then that babel-plugin-inferno is well working for me.
i add below the main config i use with gatsby:
if (process.env.NODE_ENV === `inferno`) {
requiredPlugins.unshift(babel.createConfigItem([resolve(`babel-plugin-inferno`),{import:true,pragma: "h"}], {
type: `plugin`
}));
}
const requiredPresets = []; // Stage specific plugins to add
if (stage === `develop`&&process.env.NODE_ENV !== `inferno`) {
requiredPlugins.push(babel.createConfigItem([resolve(`react-hot-loader/babel`)], {
type: `plugin`
}));
} // Fallback presets/plugins
const fallbackPresets = [];
const fallbackPlugins = [];
let targets;
fallbackPlugins.push(babel.createConfigItem([resolve(`@babel/plugin-external-helpers`)], {
type: `plugin`
}));
fallbackPlugins.push(babel.createConfigItem([resolve(`@babel/plugin-transform-flow-strip-types`)], {
type: `plugin`
}));
if (stage === `build-html`) {
targets = {
node: `current`
};
} else {
targets = {
browsers: pluginBabelConfig.browserslist
};
}
fallbackPresets.push(babel.createConfigItem([resolve(`@babel/preset-env`), {
loose: true,
modules: false,
useBuiltIns: `usage`,
targets
}], {
type: `preset`
}));
fallbackPresets.push(babel.createConfigItem([resolve(`@babel/preset-react`), {
useBuiltIns: true,
pragma: "h",
development: stage === `develop`
}], {
type: `preset`
}));
In my webpackconfig i use alias:
const res=webpackconf.resolve;
res.alias["inferno"]="inferno/dist/index.dev.esm.js";
res.alias["inferno-create-element"]="inferno-create-element/dist/index.dev.esm.js";
res.alias["inferno-create-class"]="inferno-create-class/dist/index.dev.esm.js";
res.alias["inferno-hydrate"]="inferno-hydrate/dist/index.dev.esm.js";
res.alias["inferno-shared"]="inferno-shared/dist/index.dev.esm.js";
res.alias["inferno-vnode-flags"]="inferno-vnode-flags/dist/index.dev.esm.js";
res.alias['react-dom/server']='inferno-server/dist/index.dev.esm.js';
res.alias['react']='inferno-compat/dist/index.dev.esm.js';
res.alias['react-dom']='inferno-compat/dist/index.dev.esm.js';
i found a bug from a code like that:
Nota for SideBarContext I use well create-react-context
<SideBarContext.Consumer>
{context => (
<>
{this.props.location.pathname !== "/" ? (
<Button onClick={() => window.history.back()} icon="arrow_back" flat
type="material"/>
) : (
<Button onClick={() => context.toggleDrawer(true)} icon="menu" flat
type="material"/>
)}
</>
)}
</SideBarContext.Consumer>
in looking in the source code:
Uncaught TypeError: Cannot read property '$LI' of null
at findDOMfromVNode (Users/simon/gatsby/material-starter- master/node_modules/inferno/dist/index.dev.esm.js:138)
function findDOMfromVNode(vNode) {
var flags;
var children;
while (vNode) {
flags = vNode.flags;
if (flags & 2033 /* DOMRef */) {
return vNode.dom;
}
children = vNode.children;
if (flags & 8192 /* Fragment */) {
vNode = vNode.childFlags === 2 /* HasVNodeChildren */ ? children : children[0];
}
else if (flags & 4 /* ComponentClass */) {
vNode = children.$LI;
}
else {
vNode = children;
}
}
return null;
}
it's appear that the code didn't recognize the fragment and children is null for here
children.$LI;
If i change the fragment to normal div i ve got no issue at all
ok again something to read:
I found a very strange behavior with inferno;
A input field builded with reactComponent who has got a type="password" don't event any onChange, if i change the field to a type="text" onChange then is working
I thought was my code but in normal mode (without inferno) i ve not this issue
Ok i found the problem
in inferno-compat:
change line if (!type || type === 'text') {
to if (!type || type === 'text' || type === 'password') {
I found a very strange behavior with inferno;
A input field builded with reactComponent who has got a type="password" don't event any onChange, if i change the field to a type="text" onChange then is working
That was fixed by recent PR c7113bc
babel config:
[resolve(
babel-plugin-inferno),{import:true,pragma: "h"}]
This will not work as you expect, because inferno JSX elements are compiled into different calls and not createElement, See https://github.com/infernojs/babel-plugin-inferno#pragma
<SideBarContext.Consumer>
{context => (
<>
{this.props.location.pathname !== "/" ? (
<Button onClick={() => window.history.back()} icon="arrow_back" flat
type="material"/>
) : (
<Button onClick={() => context.toggleDrawer(true)} icon="menu" flat
type="material"/>
)}
</>
)}
</SideBarContext.Consumer>
I have not seen that kind of function callback API, so I guess that's the issue here.
ok nice that you fixed input type in c7113bc (even if it's still not released in your last version)
About pragma, i adjust it just to avoid some bad output in source code.
The Fragment problem is still here and even without pragma this code upper SideBarContext.Consumer is not working
even the simplest React component wrapped around fragment will still return anything in render.
Regards
I found a bad behaviour using Fragment with inferno,
This work
<>
<App>
<Comp>
</>
But if i use fragment the behaviour is that all the components inside the fragments unmount (destroy) when i do a push history,
This behaviour conclude me to not use at all Fragment because i loose lot of thing (like csstransition)
so for now i recommand to turn all Fragment to div container
@simonjoom I cannot reproduce the issue you are having. ( Single component inside fragment )
Can you check this file what test case is missing?
https://github.com/infernojs/inferno/blob/master/packages/inferno/__tests__/fragments.spec.jsx
This example
<Sidebar.Consumer/>
Will not work in InfernoJS because it needs new context API.
I might know what you mean with Router issue. So all components there re-mount right? I will look into that tomorrow, its most likely about normalization.
Thanks for the help!
yes all remount
but this behaviour is certainly due the fact that i use a special component from gatsby that i bit modified for my case.
However with normal react i have not this unmount.
It's happened only after a change after state who modify the children components inside :
I don't put all the class here because it's very complicate but it's like a component who (re)render a component depending location change.
something like
class PageRenderer extends React.Component {
componentWillMount(){
this.wrappedPage= apigatsby(window.location)
}
componentDidMount(){
this.wrappedPage.then((end)=>{
this.setState({wrappedPage:end})
})
}
componentDidUpdate(nextprops, prevState) {
if(nextprops.location!==this.props.location){
this.wrappedPage.then((end)=>{
this.setState({wrappedPage:end})
})
}
}
render() {
return this.state.wrappedPage
}
}
Maybe it's can help you to figure out or replicate the issue
the result of wrappedPage is a react.component who contain in render something like
render(){
return(<>
<App/>
<Test />
</>)
}
with Test:
class Test extends Component {
componentWillUnmount() {
console.log("unmountTest");
}
render() {
return <div>Test</div>;
}
}
I don't see any "unmountTest" on console when i run with react, with inferno i see "unmountTest"
All the tree seems to reload
And yes tested without fragment with inferno i don't see any "unmountTest"
About
"<Sidebar.Consumer/> " it's working for me i use many kind of consumer like that and it's working great.
I use @reach/router who use context api too it's working.
just the thing is to use the polyfill create-react-context
@simonjoom I have fixed the re-mount issue with Fragments, thanks for reporting it! Commit link here:
0de1223
even the simplest React component wrapped around fragment will still return anything in render.
For that use case I'm still missing steps to reproduce, do you know how to reproduce it?
Okay, the issue occurs only when using createElement API, which means your babel plugin was not correctly configured, this is bug anyway and needs to be fixed. You can look into compiled output and if it uses createElement calls its not using inferno babel plugin.
I'm closing this ticket as Inferno v6 is now released! 🎉
@simonjoom if you still experience issues with Fragments you can create new Github ticket and we can continue there.
cool thanks.
To see my project working on live with gatsby & inferno v6.0.0
it's here: https://www.ski-courchevel.deals :) (in developement)
I'm closing this ticket as Inferno v6 is now released! 🎉
@simonjoom if you still experience issues with Fragments you can create new Github ticket and we can continue there.