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 support #425

Closed
sebastianzebrowski opened this issue Mar 18, 2014 · 31 comments
Closed

JSX support #425

sebastianzebrowski opened this issue Mar 18, 2014 · 31 comments

Comments

@sebastianzebrowski
Copy link

Any plans to support JSX (http://facebook.github.io/react/jsx-compiler.html)? I've tried using --e4x but still some files are getting wrong formatting.

@bitwiseman bitwiseman added this to the Future milestone Mar 19, 2014
@bitwiseman
Copy link
Member

No plans at this time. Feel free to take a swing at it.
If you do, be sure to include tests, and if possible a python translation.

@jdolitsky
Copy link

+1 !

@ken-okabe
Copy link

+1

3 similar comments
@schickling
Copy link

+1

@rseymour
Copy link

+1

@ftorre104
Copy link

+1

@bitwiseman
Copy link
Member

Okay, so it's popular. Someone would make a lot of friends by implementing this.

@tommcgurl
Copy link

+1

4 similar comments
@deniss-muhla
Copy link

+1

@jljorgenson18
Copy link

+1

@unbalancedparentheses
Copy link

+1

@snario
Copy link

snario commented Feb 23, 2015

+1

@metasansana
Copy link

Anyone actually tried yet?

@bitwiseman
Copy link
Member

Nope. Everyone wants it, no one wants to do it. 😄

I will give you fair warning, I'm not even sure where it would fit in the beautifier right now. If you want to take a swing at it, we should chat a bit about how you'd plan to do it.

@metasansana
Copy link

I would not even know where to start, sorry!

I just renamed my react files to .jsx and started using a vim extension for jsx instead. Now that I think about it, js-beautify probably does not need to support jsx unless it becomes an es(n) standard or something.

A seperate module dedicated to jsx might be more feasible me thinks.

@bitwiseman
Copy link
Member

I'd be happy if someone wanted to create that module as part of the beautify-web group.

@jaxbot
Copy link

jaxbot commented Mar 14, 2015

The -X option seems to process JSX just fine using this test case:

http://prettydiff.com/unit_tests/beautification_javascript_jsx.txt

On this test case, though, it reindents the file with 4 spaces instead of 2 and thus the untouched XML is formatted wrong:

https://raw.githubusercontent.com/jaxbot/reacterest/master/js/components/pin.js

Not sure why the indent is different between those two...

PrettyDiff seems to have a solution figured out, though: Glavin001/atom-beautify#144 (comment)

@bitwiseman
Copy link
Member

@jaxbot the indents are the indent_size setting (--indent-size option). Set that to 2 and you should be all set... Hmm...

It looks like http://prettydiff.com/unit_tests/beautification_javascript_jsx.txt is almost correct as well. The only section that looks really wrong is:

Input:

var Mist = React.createClass({
    renderList: function () {
        return this.props.items.map(function (item) {
            return <ListItem item={return <tag>{item}</tag>} key={item.id} />;
        });
    }
});

Output:

var Mist = React.createClass({
    renderList: function() {
        return this.props.items.map(function(item) {
            return <ListItem item = {
                return <tag>{item}</tag>
            }
            key = {
                item.id
            }
            />;
        });
    }
});

Hmm... maybe not as far from basic support as I first thought.

@jaxbot
Copy link

jaxbot commented Mar 15, 2015

@bitwiseman When I set indent size, it works fine. But I was curious why the ident size worked fine with no changes on the one example but failed on mine. Ah well.

Is your output with or without ES4X on?

@bitwiseman
Copy link
Member

Mine is with E4X on.

The reason it worked fine is one example is already indented at 4 spaces. Inside the xml sections js-beautifier treats everything as a literal, it does no reformatting yet. It just understands enough to not break e4x/jsx, not to do good things with it.

@jaxbot
Copy link

jaxbot commented Mar 15, 2015

But the bottom of the file is indented with 2 spaces and it still works, so I was confused by the indentation of the other pieces (not XML enclosed) matched the original 2. Ah well. Looks petty close, just needs some logic fixing in the XML escaping

bitwiseman added a commit that referenced this issue Mar 16, 2015
bitwiseman added a commit that referenced this issue Mar 16, 2015
@bitwiseman
Copy link
Member

If you have examples of the issues with xml escaping, please file an issue for it.

@InterStelios
Copy link

+1

1 similar comment
@godd9170
Copy link

godd9170 commented Apr 8, 2015

+1

@bitwiseman bitwiseman added this to the v1.6.0 milestone Apr 8, 2015
@bitwiseman bitwiseman removed this from the Future milestone Apr 8, 2015
@bitwiseman
Copy link
Member

@godd9170 @stelioskiayias @jaxbot @metasansana @sankargorthi @jdolitsky @schickling @rseymour @ftorre104 @tommcgurl @unbalancedparentheses @snario

You have all expressed interest in this feature and I'd like to reach the level of "non-breaking" support as part of the next release. That is, given properly formatted JSX, the beautifier does not break it. To do that I need your help.

  • Go to http://jsbeautifier.org/
  • Check the "Support e4x/jsx syntax" checkbox in the upper right.
  • Paste some JSX code into the edit box
  • Click "Beautify"
  • Take the beautified code and make sure it still works
  • If you find a code that doesn't work after beautification: get a minimal repro case of the problem and add a comment to this issue (input and actual output).
  • If you find code that reformats "oddly", but still works: get a minimal repro case of the problem and create a new issue showing the behavior (input, expected output, actual output). See the known issues below.

KNOWN ISSUES:

// This remains badly indented
var a = <x>
<note>everything {inside} the xml tags is treated as a literal string. /sadface</note>
</x>
  • Invalid xml causes beautifier to ignore rest of file. This is by design.
var qwer = <DropDown> some stuff</DropDown>; render(dropdown);
// The next line will reformat as expected.
if (true) { code.should.reformat(); and.does(); }

var qwer = <Dropdown> some stuff</DropDown>; render(dropdown);
// "Dropdown" != "DropDown", the rest of this file will not be treated as a literal string.
if (true) { code.should.reformat(); but.does.not(); }

Thanks!

@godd9170
Copy link

godd9170 commented Apr 9, 2015

Thanks for digging into this @bitwiseman!

My JSX definitely didn't break after the beautification so that's awesome. There are a few formatting things, and I've made issues for them as you've asked. (#667 and #668)

Cheers!

@bitwiseman
Copy link
Member

Cool, thanks! Anyone else?

@jaxbot
Copy link

jaxbot commented Apr 16, 2015

Worked on all my test cases! Did hit some odd stuff related to #667 but that was fixed after correctly setting my indent level.

@CumpsD
Copy link

CumpsD commented May 14, 2015

Worked for my little JSX sample :)

@CumpsD
Copy link

CumpsD commented May 14, 2015

Found this in settings:

image

But how do I actually enable it for .jsx files?

@nivv
Copy link

nivv commented May 19, 2015

Worked for my example too! Although if you disable e4x then run beautify and then enable and run it again it doesn't reformat the xml in the render block correctly. I guess that's not a bug though!

Also 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests