acornjs / acorn-jsx

Alternative, faster React.js JSX parser
MIT License
648 stars 74 forks source link

Possible token error #28

Closed nzakas closed 9 years ago

nzakas commented 9 years ago

When parsing this code:

<a>{/* this is a comment */}</a>;

The token for } appears to have an incorrect range:

  { type:
     { label: '}',
       keyword: undefined,
       beforeExpr: false,
       startsExpr: false,
       isLoop: false,
       isAssign: false,
       prefix: false,
       postfix: false,
       binop: null,
       updateContext: [Function] },
    value: undefined,
    start: 4,
    end: 28 }

With a range of [4, 28], calling code.slice(token.start, token.end) includes the comment before the }. Is this intentional?

nzakas commented 9 years ago

A little investigation: I tried a few other patterns that are similar:

if (foo) { /* foo */ }
export { foo /* foo */}

And the comment is not included with the }, so definitely contained within the JSX logic. I'll take a look on Monday.

nzakas commented 9 years ago

Is there a way to test token output already? I can't seem to find anything in the repo.

RReverser commented 9 years ago

What do you mean by "test token output"? Like, in unit tests?

nzakas commented 9 years ago

I mean, is there a way to pass a string to a function, and validate that the tokens created are as expected? test/testFail only seem to work for ASTs. I don't see any tests that look specifically at tokens.

RReverser commented 9 years ago

Sure, see https://github.com/marijnh/acorn/blob/master/test/tests.js#L28845-L28924

nzakas commented 9 years ago

Sweet, should have a PR ready soon. Thanks!

RReverser commented 9 years ago

Thank you!