JSXText nodes are mislabeled as Literal nodes

[azz]

What version of TypeScript are you using?

2.3.2

What version of typescript-eslint-parser are you using?

a294afa

What code were you trying to parse?

<a> </a>

What did you expect to happen?

Program.body[0].expression.children[0] should be a JSXText node.

What happened?

Program.body[0].expression.children[0] is a Literal node.

{ type: 'Literal',
  range: [ 3, 4 ],
  loc: { start: { line: 1, column: 3 }, end: { line: 1, column: 4 } },
  value: ' ',
  raw: ' ' }

prettier/prettier#1558

[soda0289]

I could be wrong. But doesn't the JSX AST Spec say that it must be a literal:
https://github.com/facebook/jsx/blob/master/AST.md

Does babel do something different? I think JsxText is just used as a token type and not a node type

[bakkot]

Yeah, babylon puts a JSXText node, looks like.

I'm guessing the spec is outdated or wrong. Unless people are using something other than babel to compile their JSX?

[soda0289]

Using ASTExplorer I noticed that parsers such as babel-eslint and espree both use Literal as the node type. Both of these parser are used by ESLint and linting rules are expecting this AST tree. If we were to change it then it would be best to change these parsers as well and update any ESLint rules. Babylon v5 used Literal but newer versions use JSXText so its possible there was a change in spec.

I will try to dig into this more. @JamesHenry any idea why newer versions of babel have started using JSXText instead of Literal.

[soda0289]

facebook/jsx#50
jquery/esprima#1467

Seems like there has been discussions about changing the AST psec

[Jessidhia]

That's probably a mislabeling, I'm not sure what a Literal is; the spec as it is could allow for a NumericLiteral but the contents are definitely not numeric; could allow for a TemplateLiteral but that definitely does not work without wrapping in an expression container.

The use of JSXText instead of StringLiteral (the only literal subtype close to approximating the right behavior) is probably because it is not possible to escape text inside JSXText. A StringLiteral also has a quote style, but the text inside JSX does not have (delimiter) quotes.

[bakkot]

@Kovensky ESTree has a Literal type; that's what it is.

[Jessidhia]

So it would allow for literally anything (including a regexp) inside the tags, even though that's not the actual or intended behavior.

[vjeux]

cc @sebmarkbage

[vjeux]

We just changed the JSX spec to use JSXText instead of Literal as this is what all the popular parsers are doing. I think we should follow suite in typescript-eslint-parser. facebook/jsx#50

[soda0289]

@vjeux Nice. This is a better choice and makes us align with other parsers. Will make the change this weekend. Thanks for taking the time to make it happen

[soda0289]

@vjeux Is a JSXText node going to be defined as well.

Flow uses:

interface JSXText {
  type: 'JSXText'
  value: string,
  raw: string
}

[vjeux]

@soda0289 oh yeah you are totally right. Would you mind sending a PR on the JSX repo to add it?

[soda0289]

No problem

[azz]

facebook/jsx#50 is closed. Is there much work to do on this side?

[soda0289]

The change should be straight forward. I think we should convert CRLF line endings for the value property, similar to what esprima does. It will take some time to hit a new release since I need to ensure it will work with current eslint rules. Ill start looking into later today.

[soda0289]

We can make this change. There are lots of rules that this will break and eslint may not be willing to make the change until v5. This means the JSXText node type will not be enabled by default but can be used by enabling a flag. This should allow prettier to use the same node type as other parsers and give eslint and rule authors the time to make the adjustments they need. Will post a PR in the next few days.

[sompylasar]

@soda0289 What stops this from being merged and released?

This results in false positive on react/no-unescaped-entities which triggers on whitespace with newlines.
https://github.com/yannickcr/eslint-plugin-react/blob/f0dcaca92418bb791f585e867d720a96dd43a277/lib/rules/no-unescaped-entities.js#L72

Example:

<SomeComponent>
  {children}
</SomeComponent>

gets parsed as:

JSXElement: '<SomeComponent>'
  Literal: '\n  '
    ...

and tells me to escape the newline character.

[soda0289]

@sompylasar
This problem is about the node type Literal being used inside JSXElement children. It will be a breaking change that will produced an unexpected AST for eslint-plugin-react. The issue that you are talking about is different. Could you open a new issue?

[soda0289]

@sompylasar I'm pretty sure it's becuase the Literal node has the incorrect loc property. Both line number and column number are incorrect

[sompylasar]

@soda0289 #278