acornjs / acorn-jsx

Alternative, faster React.js JSX parser
MIT License
646 stars 72 forks source link

Unexpected character error on blocks within functions within {} in JSX #9

Closed sverrejoh closed 9 years ago

sverrejoh commented 9 years ago

acorn-jsx fails if you have a block within a function within a { } expression, like in the example below.

❯ cat index.jsx
var foo = <div> {
  function() {
    if (true) {
    }
  }()
}
</div>;                                                                                                     

❯ jsx index.jsx                     
built Module("index.jsx")
var foo = React.createElement("div", null, " ", 
  function() {
    if (true) {
    }
  }()

);

❯ acorn-jsx index.jsx 
Unexpected character '
' (4:5)

~/Projects/Testcases/6to5-react  
❯ 
RReverser commented 9 years ago

No doubts it's a bug, but why are you putting such a complex expressions inside JSX container? It's not meant to be used that way.

sverrejoh commented 9 years ago

Thanks for the feedback! This was just a silly example to show the bug.

I guess the rationale is that a self calling function expression can look better than the suggested use of ternary if's.

<div>
{
function() {
  if (foo) {
    return <div/>;
  } else if (bar) {
    return <div/>;
  } else if (baz) {
    return <div/>;
  } else {
    return <div/>;
  }
}()
}
</div>

VS.

<div>
{
  foo? <div>: bar? <div/>: baz?: <div>: </div> ... etc.
}
</div>

Another slightly better example, would be the use of filter:

var a = [1,2,3,4];
var foo = <div> {
  a.filter(function(item) {
    if (item == 2 || item == 3) {
      return true;
    } else {
      return false;
    }
  })
}
</div>;

Again you could just return the bool, but don't you think there is times where this is useful?

RReverser commented 9 years ago

Looks like all those issues started with acceptance of #1. Removing inXJSChildExpression fixes all those kinds of issues but skipped whitespace, and in general expressions inside containers should know nothing about JSX context they're in. I'll look into better ways of handling the original issue.

RReverser commented 9 years ago

Fixed now, thanks. Let me know if you find edge cases that this could break (handling whitespaces differently is pretty tricky).