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

jsx-curly-spacing autofix deletes comments #648

Closed
lencioni opened this issue Jun 23, 2016 · 6 comments
Closed

jsx-curly-spacing autofix deletes comments #648

lencioni opened this issue Jun 23, 2016 · 6 comments
Labels

Comments

@lencioni
Copy link
Collaborator

lencioni commented Jun 23, 2016

If I have some JSX like:

<div>
  { /* comment */ }
</div>

the autofixer will change it to:

<div>
  {}
</div>

but it should change it to:

<div>
  {/* comment */}
</div>
@ljharb ljharb added the bug label Jun 23, 2016
@ahstro
Copy link

ahstro commented Nov 30, 2016

Title should probably be changed to "jsx-curly-spacing doesn't account for comments" since things like {something /* comment */} triggers an error.

@s-h-a-d-o-w
Copy link
Contributor

@yannickcr Your commit unfortunately doesn't seem to fix this issue. The example provided by @lencioni still produces the buggy result of removing the comment (tested using 7.3.0).

So I would argue that this issue should be re-opened.

@ljharb
Copy link
Member

ljharb commented Sep 6, 2017

The commit includes automated tests that should verify it's working.

Can you open a PR that includes a failing test case?

@s-h-a-d-o-w
Copy link
Contributor

Was that what you had in mind?

@ljharb
Copy link
Member

ljharb commented Sep 12, 2017

Thanks, that's appreciated! I'll take a look soon.

@s-h-a-d-o-w
Copy link
Contributor

Cool, thanks for the update!
(Essentially my first PR on a major project, so I just wanted to make sure I did it properly and understood you correctly :) )

ljharb pushed a commit to s-h-a-d-o-w/eslint-plugin-react that referenced this issue Dec 20, 2017
ljharb pushed a commit to s-h-a-d-o-w/eslint-plugin-react that referenced this issue Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants