Add babel-plugin-transform-react-inline-elements about react-slingshot

[halayli]

The documentation for this plugin is slightly incomplete. The reason I had to include it in my babelrc after upgrading to babel6 is to be able to assign jsx elements to jsx attributes. Otherwise, I'd get errors of this sort:

Module build failed: TypeError: App.js: Property value of JSXAttribute expected node to be of a type ["JSXElement","StringLiteral","JSXExpressionContainer"] but instead got "CallExpression"

For such code:

<Tab value="dashboard"
   icon={<FontIcon className="material-icons">dashboard</FontIcon>}
   label="DASHBOARD"
/>

Removing icon= fixes it.

[coryhouse]

@halayli Per the docs it should only be enabled in production, so how would this solve your problem in dev?

[halayli]

I had to enable in babelrc's development env. I couldn't find any other solution for now. This problem only showed up after I upgraded to babel 6.

[coryhouse]

Ouch. Given the negative development impact, I definitely can't wire it up in dev. I've never seen this pattern before. Could you nest FontIcon as a child of Tab instead?

[halayli]

This is a very common pattern in material-ui and ui-related libs in general. It's a handy pattern. I am not sure FontIcon can nest in the child but there are plenty other components that have other attributes taking jsx elements.

Given it's implications I understand why you wouldn't add it to dev. But I wonder how it was working in babel 5.x.

[barrystaes]

@halayli you say

to be able to assign jsx elements to jsx attributes

Weird works for me .. i do this all the time, just not inline but using a variable:

const iconJsx = <FontIcon className="material-icons">dashboard</FontIcon> 
return <Tab value="dashboard" icon={iconJsx} label="DASHBOARD"/>

[halayli]

@barrystaes it works with variables but not when you assign directly to icon.

[barrystaes]

TLDR; I think the strength of react-slingshot is being complete but not complicated.
I dont think this addition makes it that much more complete to warrant the additional complexity. 👎

Long version; @coryhouse asks

Potential performance enhancement: https://babeljs.io/docs/plugins/transform-react-inline-elements/
Need to test and decide if the benefits justify the downsides. Any opinions?

I also vote against using it in react-slingshot.
From my viewpoint and that of a new user, i think this just complicates things (sourcecode readability, output code readability, debugability) but does not add value / solve a problem that i experience or even know of.

[coryhouse]

Gotta agree with you @barrystaes. Thanks for the input! Until there's an alternative without the big downsides, this will have to stay out of the kit. Going to go ahead and close this as "won't fix" since there's nothing actionable we can do at this time.