Eslint-plugin-react: jsx-curly-brace-presence warns incorrectly on trailing whitespace

[taion]

This is a regression from #2409, cc @vedadeepta @ljharb

Per https://github.com/yannickcr/eslint-plugin-react/pull/2409#discussion_r330147476, when running autofix with this change, I now get e.g.

           <div>
-            Confirm{' '}
+            Confirm
             <button type="button" onClick={this.onClickYes}>

The rule may be correct in isolation, but this interacts very poorly with the standard automatic removal of trailing whitespace.

[vedadeepta]

@taion you want to keep the traling white space iff it is inside curly braces ?

I think the rule can be adjusted for this special case but i'm not sure if markup should be written like this though.

[taion]

jsx-curly-brace-presence should not warn when the curly braces surround trailing whitespace.

How else would you write this markup? In general it's not that odd to put a line break in this sort of place. It's e.g. what Prettier does.

[ljharb]

That also makes total sense.

[ljharb]

@vedadeepta indeed, putting whitespace inside curlies is the only way to write it so that HTML doesn't collapse it.

[vedadeepta]

@taion makes sense to account for your case, earlier i was thinking more in terms of css paddings/margins for creating spaces.

[ryansully]

I think I'm experiencing a related situation where I need whitespace between children, but the whitespace child warns incorrectly.

<button>
  <i className="fa fa-check" />
  {' '}
  Confirm
</button>

In this case I'm also using jsx-one-expression-per-line, so it's not just an issue of trailing whitespace, but of any use of whitespace children.

There is also the case if there needs to be non-whitespace text with leading or trailing whitespace, or both:

<p>
  {'The '}
  <strong>rest</strong>
  {' of the '}
  <a href="/">site</a>
  {' already.'}
</p>

An autofix of this results in joined words with only one space in the middle.

[taion]

This still led to a slightly unsafe autofix in

--- a/www/src/examples/Navbar/Brand.js
+++ b/www/src/examples/Navbar/Brand.js
@@ -28,7 +28,7 @@
         height="30"
         className="d-inline-block align-top"
       />
-      {' React Bootstrap'}
+      React Bootstrap
     </Navbar.Brand>
   </Navbar>
 </>;

But I think I'm okay with moving all that whitespace to trailing.

Thanks!

[taion]

And also:

-            <React.Fragment key={idx}>
-              {' / '}
-              {crumb}
-            </React.Fragment>
+            <React.Fragment key={idx}>/{crumb}</React.Fragment>

So this is a rule in the sense that both this and the above are things that I'd want to fix.

But in both cases the autofix ate my whitespace, which is not ideal.

[ljharb]

Both i consider bugs; could you file a new issue, or a PR with the test cases?

[SirCameron]

Has a new issue been filed? Why not reopen this issue?

[ljharb]

It’s cleaner to avoid an issue being “closed” by multiple PRs, and it helps ensure that further comments don’t get lost by being in a closed issue.

[taion]

Sorry, I've been meaning to put together at least the failing test, but I've been jammed for time. Let me open a new issue at least.

[taion]

ref https://github.com/yannickcr/eslint-plugin-react/issues/2454