-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 warns incorrectly on trailing whitespace #2427
Comments
@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. |
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. |
That also makes total sense. |
@vedadeepta indeed, putting whitespace inside curlies is the only way to write it so that HTML doesn't collapse it. |
@taion makes sense to account for your case, earlier i was thinking more in terms of css paddings/margins for creating spaces. |
I think I'm experiencing a related situation where I need whitespace between children, but the whitespace child warns incorrectly.
In this case I'm also using There is also the case if there needs to be non-whitespace text with leading or trailing whitespace, or both:
An autofix of this results in joined words with only one space in the middle. |
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! |
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. |
Both i consider bugs; could you file a new issue, or a PR with the test cases? |
Has a new issue been filed? Why not reopen this issue? |
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. |
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. |
ref #2454 |
This is a regression from #2409, cc @vedadeepta @ljharb
Per #2409 (comment), when running autofix with this change, I now get e.g.
The rule may be correct in isolation, but this interacts very poorly with the standard automatic removal of trailing whitespace.
The text was updated successfully, but these errors were encountered: