Skip to content
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

v0.14.3: Inside render's return, any comments placed above the first line cause various errors #5573

Closed
sheminusminus opened this issue Nov 30, 2015 · 3 comments

Comments

@sheminusminus
Copy link

Using v0.14.3, one comment placed on first line of render()'s return causes nothing to be rendered in browser. My code causing this:

    return (
        {/* class is a reserved word, so must ref this with className */}
        <div className="wrapper">
            <h1>React!</h1>
            {/* attributes defined in the ReactDom.render() function are referenced with { this.props.attribute } */}
            <h2>{ this.props.message }</h2>
            <Thingy name="Thingy1" content="Cat in the Hat" />
            <Thingy name="Thingy2" content="Bat in the Frat" />
            <Thingy name="Thingy3" content="Sat in the Vat" />
        </div>
    )

Two comments cause an unexpected token error. My code causing this:

    return (
        {/* react needs one parent for the component */}
        {/* class is a reserved word, so must reference this with className */}
        <div className="wrapper">
            <h1>React!</h1>
            {/* attributes defined in the ReactDom.render() function are referenced with { this.props.attribute } */}
            <h2>{ this.props.message }</h2>
            <Thingy name="Thingy1" content="Cat in the Hat" />
            <Thingy name="Thingy2" content="Bat in the Frat" />
            <Thingy name="Thingy3" content="Sat in the Vat" />
        </div>
    )

This causes no errors at all:

    return (
        <div className="wrapper">
        {/* react needs ONE parent container for the component */}
        {/* also, class is a reserved word in JSX, so must ref this with className */}
            <h1>React!</h1>
            {/* attributes defined in the ReactDom.render() function are referenced with { this.props.attribute } */}
            <h2>{ this.props.message }</h2>
            <Thingy name="Thingy1" content="Cat in the Hat" />
            <Thingy name="Thingy2" content="Bat in the Frat" />
            <Thingy name="Thingy3" content="Sat in the Vat" />
        </div>
    )
@zpao
Copy link
Member

zpao commented Nov 30, 2015

Looks like both your first and second snippets are resulting in parse errors. At least they are for me when I drop them into a React component in the Babel repl (http://babeljs.io/repl/). I haven't tested with anything else (perhaps Babel 6 behaves slightly differently, though I don't think it's supposed to). Maybe you're using something else to parse?

This is unfortunately one of those areas where JSX gets weird. Since it's all a bit made up, there are edge cases with the behavior, and maybe we can define things a bit better.

I'll walk through each of these though and try to explain why they're parse errors and why the last one works.

1, 2

These are both parse errors for the same reason. It boils down to not knowing that you're in a piece of JSX when you put the comments inside the braces. At that point the parser hasn't switched over the treating the block as JSX (it hasn't encountered something it believes is an opening tag) so it thinks you just have an object. { /* comment */ } is just parsed as {}. So stepping past that, it does see some JSX but it doesn't matter because it ultimately looks like this: return {}React.createElement(...) which is going to be a parse error. #2 is just an extension of that, except you have 2 empty objects.

3

For this one, the opening <div> indicates to the parser to start treating {...} as something besides objects and to drop the expression in there. {/* comment only */} blocks are removed because that's what we decided would make sense when we introduced comments. {'foo'} will drop the literal in there instead.

I'm sorry that you hit one of the cases where JSX breaks down but hopefully that clears it up for you. There isn't much we can really do. We don't really own the tooling anymore and unfortunately since we're building this on top of JS, until the parse sees something to indicate JSX, it has to assume you're still in JS.

@zpao zpao closed this as completed Nov 30, 2015
@sheminusminus
Copy link
Author

Thanks so much for the explanation and quick response! It makes sense though, now that you explain it.

I know you guys are probably busy, but if you have a sec to check it out, I was talking about this with one of my professors and he was wondering if using an explicit JSX declaration could potentially fix the issue? Something along the lines of:

var Foo = React.createClass({
renderType: ‘JSX’,
render: function() {
return (
{/* could this work? */}
...

@jimfb
Copy link
Contributor

jimfb commented Dec 3, 2015

@emkolardev Probably not; specifying a renderType on the class spec would not be sufficient to disambiguate the grammar.

It would be trivial to disambiguate if you had some obvious start and end token:

var Foo = React.createClass({
render: function() {
return (jsx```
{/* could this work? */} 
...
```jsx
...

But since JSX is just javascript with a couple extra token types, the {/* could this work? */} is effectively just an empty floating block which is not legal in javascript. The empty block is too similar to other javascript blocks (like if(true) {/* could this work? */}) for us to have a reasonable chance at disambiguating them.

Ultimately, one could write a parser that used some clever backtracking and heuristics to "figure it out", but there is something to be said for managing complexity and keeping something reminiscent of a LL Grammar. If you are interested in this stuff, you should take a class on compilers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants