search

royriojas / esformatter-jsx

esformatter plugin: format jsx files (or js files with Facebook React JSX Syntax)
MIT License
142 stars 25 forks source link

New option: closingTagOnNewLine #86

Closed dreel closed 7 years ago

dreel commented 7 years ago

Fix for #67 .

If this option is true, multiline tags will have the closing tag on its own line.

Instead of: 

<div
  attr1='foo'
  attr2='bar'>
  A div.
</div>

You get:

<div
  attr1='foo'
  attr2='bar'
>
  A div.
</div>

This is not thoroughly tested, but works in my sample cases.

royriojas commented 7 years ago

Hi @dreel, thanks for the PR. It seems the PR failed the test. Ideally this should be easy to fix by running the beautification of the files with npm run beautify.

Will take some time tomorrow to review and merge. Would you mind adding a simple unit test for this case? check the unit tests folder to see how the tests are done. Thanks in advance.

dreel commented 7 years ago

Hi @royriojas , I also found an edge case where this doesn't work as expected (a self-closing multiline tag, it should carry the /> instead of just the > to the next line). I will fix try to take a look at adding a unit test and fixing that case over the weekend.

royriojas commented 7 years ago

hi @dreel, I really appreciate you take the time to fix that. Thank you man.

dreel commented 7 years ago

Fix and unit test is in, and I had to fix bug-34.js as well, which I think was unrelated to my changes. I'm guessing it was an update through npm that caused the tests to start complaining about the collision on the export of some.

royriojas commented 7 years ago

nice @dreel will merge