Skip to content
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

react/jsx-curly-brace-presence "children" "always" complains about whitespace #1727

Closed
dwilt opened this issue Mar 16, 2018 · 13 comments · Fixed by #2521
Closed

react/jsx-curly-brace-presence "children" "always" complains about whitespace #1727

dwilt opened this issue Mar 16, 2018 · 13 comments · Fixed by #2521

Comments

@dwilt
Copy link

dwilt commented Mar 16, 2018

"eslint": "^4.18.2",
"eslint-plugin-react": "^7.6.1",

I have the prop: "react/jsx-curly-brace-presence":["error", { "props": "always", "children": "always" }], in my .eslintrc file because I'm trying to convert:

<div className="NameCapture_wrapper">
    <p className="NameCapture__question">
        What's your first name?
    </p>
    <NameCaptureNameInput/>
</div>

to:

<div className={`NameCapture_wrapper`}>
    <p className={`NameCapture__question`}>
        {`What's your first name?`}
    </p>
    <NameCaptureNameInput/>
</div>

but eslint is complaining about the whitespace:

  10:44  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence
  13:49  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence
  14:56  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence
  17:53  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence
  18:45  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence
  19:23  error  Need to wrap this literal in a JSX expression          react/jsx-curly-brace-presence

Below is what fixes it but I shouldn't have to put all of my JSX on one line right?

<div className={`NameCapture_wrapper`}><p className={`NameCapture__question`}>{`What's your first name?`}</p><NameCaptureNameInput/></div>
@ljharb
Copy link
Member

ljharb commented Mar 16, 2018

The bug here is that the one liner fixes it; there’s no reason you need a literal inside curly braces there.

Why are you trying to convert a jsx string literal into a JS string literal with no interpolations?

@ljharb
Copy link
Member

ljharb commented Mar 16, 2018

Actually the oneliner fixes it because it assumes that the whitespace surrounding the literal is significant, so that’s correct behavior, but the question still remains.

@dwilt
Copy link
Author

dwilt commented Mar 16, 2018

@ljharb Thanks for the reply. I wanted to keep all strings inside of curly braces in our project for consistency purposes. That way, if we were to put a variable in (interpolation), we wouldn't need to go and add the backticks and curlys after -- I just really prefer the consistency. I also think it's clearer to read as you know it's a string.

@ljharb
Copy link
Member

ljharb commented Mar 16, 2018

Consistency i buy - I’m not sure what else it’d be than a string, and you can add interpolations directly with jsx.

I think it’d be worth exploring some test cases in a PR to see what would break by adddressing this.

@dwilt
Copy link
Author

dwilt commented Mar 16, 2018

@ljharb You're correct, it would only be a string.

I wasn't aware that you are able to have interpolations directly:

const test = `blah`;

<p className={`NameCapture__title`}>
    What's {test} your first name?
</p>

Thanks for your responses!

@tranhl
Copy link

tranhl commented Feb 4, 2019

Actually the oneliner fixes it because it assumes that the whitespace surrounding the literal is significant, so that’s correct behavior, but the question still remains.

To me this doesn't seem like correct behavior, at least from what I imagined the rule to enforce. I imagined that the rule would be indentation-aware, but it doesn't seem like this is the case.

<div className={`NameCapture_wrapper`}>
    <p className={`NameCapture__question`}>
        {`What's your first name?`}
    </p>
    <NameCaptureNameInput/>
</div>

Shouldn't the above JSX produce no warnings with the following rule?

...
"react/jsx-curly-brace-presence": ["error", { "props": "always", "children": "always" }]
...

@ljharb
Copy link
Member

ljharb commented Feb 4, 2019

Yes, with “always” (a setting i have zero understanding of why anyone would want), I’d expect those to be correct.

@tranhl
Copy link

tranhl commented Feb 4, 2019

So this is a bug then. Is there anyone working on fixing this? If not, I can have a look into it.

@ljharb
Copy link
Member

ljharb commented Feb 4, 2019

Please, go for it!

@tranhl
Copy link

tranhl commented Feb 4, 2019

Upon further inspection of the available rules, it appears that you can use jsx-no-literals to lint expression containers, which is a workaround for this bug.

For example, with these rules:

...
"react/jsx-curly-brace-presence": ["error", { "props": "always", "children": "never" }],
"react/jsx-no-literals": ["error", {"noStrings": false}],
...

... this will pass

<div className={`NameCapture_wrapper`}>
    <p className={`NameCapture__question`}>
        {`What's your first name?`}
    </p>
    <NameCaptureNameInput/>
</div>

... and this will invoke errors

<div className=`NameCapture_wrapper`> {/* Needs expression container */}
    <p className=`NameCapture__question`> {/* Needs expression container */}
        What's your first name? {/* Needs expression container */}
    </p>
    <NameCaptureNameInput/>
</div>

Perhaps we could extend the jsx-no-literals rule to implement prop linting as well and deprecate this rule?

@ljharb
Copy link
Member

ljharb commented Feb 4, 2019

I think they're different enough that they should remain separate.

@rafbgarcia
Copy link
Contributor

I have the same problem:

This throws Need to wrap this literal in a JSX expression error

<div className="pb_user_badge">
  <span>{'USER BADGE CONTENT'}</span>
</div>

But this doesn't

<div className="pb_user_badge"><span>{'USER BADGE CONTENT'}</span></div>

Adding "react/jsx-no-literals": ["error", {"noStrings": false}] didn't change anything for me.

@ljharb @tranhl do you have any suggestions to get around this?

@ljharb
Copy link
Member

ljharb commented Dec 5, 2019

I would expect both to warn or not warn the same.

rafbgarcia added a commit to powerhome/playbook that referenced this issue Dec 5, 2019
I ran the auto fix for this lint but it still errors for all instances fixed, see:

jsx-eslint/eslint-plugin-react#1727 (comment)
@ljharb ljharb closed this as completed in 0f8d790 Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants