facebook / jsx

The JSX specification is a XML-like syntax extension to ECMAScript.
http://facebook.github.io/jsx/
1.95k stars 133 forks source link

Add early error to disallow invalid adjacent JSX elements #120

Open nicolo-ribaudo opened 4 years ago

nicolo-ribaudo commented 4 years ago

Currently this is perfectly valid JSX code:

<El>text</El> <g> / </g>
0

How is it possible? Adjacent JSX elemets must be wrapped inside another element, or inside a fragment. Well, those are not two JSX elements.

It's a jsx element (<El>text</El>), which is compared by < with the variable g, which is then compared by > with the regex / </g, and which is then compared by > with 0. If we add some parentheses for readability, it becomes

(((<El>text</El>) < g) > / </g) > 0

Currently JSX parsers don't handle this (valid) code well, because they all assume that the user is trying to use to JSX elements:

I propose to disallow that code since it is highly confusing, similarly to how -1 ** 2 is disallowed by the ECMAScript specification. To do so, we would need to define a new early error which applies to the RelationalExpression production:

It is an early Syntax Error if RelationalExpression matches the JSXElement < ShiftExpression or JSXFragment < ShiftExpression productions

By doing so, the spec would align with the ecosystem and allow parsers to throw errors helpful for the users, like Babel's Adjacent JSX elements must be wrapped in an enclosing tag. Did you want a JSX fragment <>...</>?

Huxpro commented 2 years ago

@nicolo-ribaudo Oh wow this is interesting. Thanks for reporting this and providing a very clear analysis.

Since we've moved the spec to ecmarkup (#135), I think we are finally in a good position to formalize errors like this. Would you mind making a PR?

Regarding the potential breaking change on the Flow parser: @pieterv, is Flow team comfortable with this change?

pieterv commented 2 years ago

Looks like this has been fixed in Flow since this issue has been created as the astexplorer link is showing the elements after the first JSX element parsed as binary expressions. Seems like a reasonable change anyway, we can fix any issues that come up.

See: https://astexplorer.net/#/gist/66ddd5a43c135ee13b7872e65de033c4/47ffc8a8b45cbbc6e8152423b9c7d5c351934288

Huxpro commented 2 years ago

@nicolo-ribaudo

I propose to disallow that code since it is highly confusing, similarly to how -1 ** 2 is disallowed by the ECMAScript specification.

How is this specified? I couldn't find any SS: early errors clause dedicated for that in the full spec or the original https://tc39.es/proposal-exponentiation-operator/ addition. I was imaging that we can specify JSXElement < similarly in #141