Update spec for putting JSXFragment inside attributes

[clemmy]

I think this is desired behavior and something that was overlooked in https://github.com/facebook/jsx/pull/93 - the idea is to allow fragments to be a part of JSX attributes. For example,<MyComponent attrib={<></>} />.

[sebmarkbage]

The PR is correct but your example is wrong. That was already allowed. This PR makes it possible to have this: <MyComponent attrib=<></> />

[clemmy]

@sebmarkbage

Ah, interesting. I wasn't aware of that - I guess Babel doesn't have that implemented yet. For example, if I try to transform <a x=<x></x>></a>, I get Property value of JSXAttribute expected node to be of a type ["JSXElement","StringLiteral","JSXExpressionContainer"] but instead got "CallExpression".

(https://babeljs.io/repl/#?babili=false&browsers=&build=&builtIns=false&code_lz=DwQwBAHgvMEHzAPTwYkcg&debug=false&circleciRepo=&evaluate=true&lineWrap=true&presets=es2015%2Creact%2Cstage-2&targets=&version=6.26.0)

[sophiebits]

We thought it would be good but then sorta never followed through. Now it seems more confusing than necessary.

[DanielRosenwasser]

TypeScript also doesn't support it. Perhaps it would be a good idea to revisit whether this should be in the spec at all. 😄

[Jessidhia]

Yeah, always putting anything that is not plain text in an expression container definitely seems saner.

But now I’m not sure if this is a bias because of how it’s “always been done”. On Fri, Oct 13, 2017 at 9:20 Daniel Rosenwasser notifications@github.com wrote:

TypeScript also doesn't support it. Perhaps it would be a good idea to revisit whether this should be in the spec at all. 😄

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/facebook/jsx/pull/94#issuecomment-336316274, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdfV3u3-fDCZAPU_hRx-B7ObesHhpmks5srqzhgaJpZM4P2URE .

[loganfsmyth]

@clemmy Babel's parser has always supported it, but the transform itself either never worked or broke at some point. We've fixed it for 7.x https://github.com/babel/babel/issues/6004

[sophiebits]

Never worked. Can we hold off on adding this and make sure we get agreement that we want to turn it on?

[loganfsmyth]

If that's what people think, sure. I just took the fact that it's in the spec at face value and figured we should fix it since it was a trivial bug causing the breakage in the first place.

[sophiebits]

It wasn't a bug, we had never gotten agreement to add it to the transform.

[loganfsmyth]

Alright, good to know. It does seem super confusing that it's in the spec text if that is the case. Should we also remove it from the spec then? Or at least mark it as deprecated and not recommended.

[sebmarkbage]

I'm in favor of just adding it and starting to support it (if we have consensus).

[clemmy]

Personally, I'm not the biggest fan of that syntax - it looks super confusing.

[DanielRosenwasser]

I spoke with folks from our team. It does seem somewhat confusing, and clearly rare if nobody has filed an issue in all the time that Babel & TypeScript have supported JSX. We'd be for removing from the spec, or putting it on a deprecation path.

[RyanCavanaugh]

I raised this earlier at #53 without any success. We still haven't implemented elements in attribute positions in TypeScript and no one has cared. Given the lack of interest from anyone other than transpiler implementers it really seems like it ought to be removed before someone does implement it and cause a syntactic ambiguity that prevents more useful syntax from being added in the future.