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
jsx-curly-brace-presence removes braces that are required #1449
Comments
The spec lists all the characters that are disallowed:
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: |
Here's a problem I ran into in a production code base: TL;DR The jsx-curly-brace-presence must either:
Escaping logicWhen going from JS to JSX, one has to:
When going from JSX to JS, one has to:
Note that I think it is important to for example convert White spaceWhite space handling is not part of the JSX spec: facebook/jsx#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: prettier/prettier#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. NotesThe babel repl is super handy for checking how Babel converts escapes. Neither quoted attribute values in JSX nor JSXText have JS-style
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 opinionI'm in favor of choosing the "bail out approach" because:
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 :( |
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. |
@jackyho112 Awesome! Ping me in your PR and I'll help reviewing. |
with the
7.4.0
release these braces are removed and then eslint chokes on the syntax.The text was updated successfully, but these errors were encountered: