Eslint-plugin-react: jsx-curly-brace-presence removes braces that are required

[modosc]

<style type="text/css">{'.mt0 { margin-top: 0; }'}</style>

with the 7.4.0 release these braces are removed and then eslint chokes on the syntax.

[lydell]

The spec lists all the characters that are disallowed:

JSXTextCharacter : SourceCharacter but not one of {, <, > or }

Note that the spec does not define any way escaping them, so the linter rule must either skip reporting and converting string literals with such characters in them to JSXText, or use the defacto standard of using HTML-style escapes supported by Babel: {, <, >, }, respectively. https://github.com/facebook/jsx/issues/4

[lydell]

Here's a problem I ran into in a production code base: <Color text={"\u00a0"}/> was auto-fixed into <Color text="\u00a0"/>, which changed the text prop from a non-breaking space to the literal string \u00a0.

TL;DR The jsx-curly-brace-presence must either:

• Bail out if a problematic character is found.
• Implement the below escaping logic.

Escaping logic

When going from JS to JSX, one has to:

• Escape everything that looks like a JSX &-escape. For example,   →  . One can either replace _all_ & characters with &, or only valid &-escapes (not &foobar;, for example).
• Replace all JS \-escapes with JSX &-escapes, or the literal unicode character. For example, \u00a0 →   (or  ), \a (useless escape) → a, \r → _actual CR_ (or 
), \n → _space!_ (Babel seems to do so at least).
• Escape all disallowed characters in JSX, depending on context: " → ", or ' → ', or {<>} → {<>}.

When going from JSX to JS, one has to:

• Escape everything that looks like a JS \-escape. For example, \n → \\n.
• Replace all JSX &-escapes with JS \-escapes. For example,   → \u00a0 (or \xa0).
• Escape quotes and line terminator: " → \" or ' → \', similarly for the line terminators: CR, LF, U+2028 and U+2029.

Note that I think it is important to for example convert \u00a0 into   (or &#a0;), and not into   (an actual non-breaking space character), since the author probably chose to use an escape instead of the real character in the first place.

White space

White space handling is not part of the JSX spec: https://github.com/facebook/jsx/issues/6. It is up to each implementation of JSX. I guess Babel is the defacto standard, but Facebook has some internal tooling with slightly different rules: https://github.com/prettier/prettier/issues/2279

Either way check out the difference in white space in the output here:

<a a="start

      end"/>
;
<a a={"start\
      \
      end"}/>

"use strict";

React.createElement("a", { a: "start end" });
React.createElement("a", { a: "start\
      \
      end" });

I haven’t had the time to think about how this should be handled yet.

Notes

The babel repl is super handy for checking how Babel converts escapes.

Neither quoted attribute values in JSX nor JSXText have JS-style \-escapes. Compare the spec with string literals in the ES spec. (A SourceCharacter is "any Unicode code point".)

&-escapes are undocumented but I guess how Babel treats them is the defacto standard.

Many of the jsx-curly-brace-presence test are wrong (the fixed code output sometimes even contain syntax errors). For example, these ones: https://github.com/yannickcr/eslint-plugin-react/blob/1f14fad95d28672f1eb16dbe962557a7072d16e9/tests/lib/rules/jsx-curly-brace-presence.js#L183-L217

My opinion

I'm in favor of choosing the "bail out approach" because:

• It is simpler.
• <style type="text/css">{'.mt0 { margin-top: 0; }'}</style> is much more readable than <style type="text/css">.mt0 &#123; margin-top: 0; &#125;</style>.

I could be interested in implementing this when we reach a consensus and I find the time.

Even though I really like this rule (big thanks to @jackyho112), I’m going to turn it off for now since it is not safe to use :(

[jackyho112]

@lydell

Thanks for the input! That was extremely helpful.

When I was implementing this, I referred mostly to the React docs. But the JSX specs are definitely better! I can’t believe I didn’t think of it at the time.

I am in favor of the bailout approach as well. I will try to get a PR up by this week to fix this.

[lydell]

@jackyho112 Awesome! Ping me in your PR and I'll help reviewing.