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

.tsx support #601

Closed
bennylut opened this issue Oct 11, 2015 · 41 comments
Closed

.tsx support #601

bennylut opened this issue Oct 11, 2015 · 41 comments
Assignees
Milestone

Comments

@bennylut
Copy link

I would like to suggest support for tsx files which are a jsx extention for typescript.

@prettydiff
Copy link
Collaborator

For some general background on JSX-like grammars: #608 (comment)

@pristas-peter
Copy link

+1

1 similar comment
@olegsmetanin
Copy link

+1

@sqrtroot
Copy link

+1 👍

@prettydiff
Copy link
Collaborator

This morning I started evaluating support for TypeScript in Pretty Diff's JS parser (jspretty.js). If I can correctly identify and parse TypeScript then TSX support is already available, as the parser won't know the difference between TSX and JSX.

I am not a TypeScript user myself so I will have a lot of testing to do to ensure I get this right, but also negative impact test against JavaScript and ES6/React support. Once I get a bit further I will start posting beta test links for people to examine so that bugs can be identified.

@prettydiff
Copy link
Collaborator

Here is something to test against: http://prettydiff.com/ignore/testlocation/index.xhtml?m=beautify

Try out some TypeScript and tsx samples at the above tool. If you find any errors please let me know. In the mean time I will be looking for additional TypeScript code samples to test against and perform some regression tests.

@davidtme
Copy link

+1

@prettydiff
Copy link
Collaborator

When looking at the code example at https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#1.9 what is <t>? Is that a type parameter? Does it always follow a reference? Could it follow an end character: ], ), }? Are spaces permitted between it and the thing it is describing?

It is a type parameter (or type parameter list) and only directly follows interface, class, and function declarations. I still need help as to whether the type parameter can have space between it and the reference it extends.

@prettydiff
Copy link
Collaborator

Bah, https://github.com/Microsoft/TypeScript/wiki/JSX#the-as-operator

Is this code creating a JSX element with the content of bar;, or is it asserting that bar is of type foo and there is an invalid expression on line 2? To simplify cases like this, angle bracket type assertions are not available in .tsx files. As a result, in a .tsx file, the previous code would be interpreted as a JSX element, and in a .ts file it would result in an error.

As a result could you guys please pass your TSX code samples through http://prettydiff.com/?m=beautify and let me know if they are beautified appropriately?

@ShMcK
Copy link

ShMcK commented Jan 18, 2016

Some issues with formatting after any dot notation. An example:

    return <button
                      onClick={this
                        .item
                        .bind(this)}>Click Here</button>;

@prettydiff
Copy link
Collaborator

@ShMcK I have an option for that: methodchain . Just give it a value of chain (Chain methods onto a line)

@ShMcK
Copy link

ShMcK commented Jan 18, 2016

That's great. But I prefer the chained methods in my logic code, and not so much in my template code.

Is it possible to get the best of both worlds when using .jsx/.tsx?

@prettydiff
Copy link
Collaborator

Not yet. I would have to create an option to supply a different setting when JavaScript is a J/TSX attribute.

@aspirisen
Copy link

+1

@digital-wonderland
Copy link

@prettydiff I run my components through your site and was happy with the result with only a few problems:

  1. chained method calls (which is probably related to what @ShMcK is describing):
    render() {
        let url = `https://placeholdit.imgix.net/~text?w=${this.props.width}&h=${this.props.height}`;

        if (this.props.text != null) {
            url += `&txt=${this.props.text.replace(/\s/, "+")}`;
            url += `&txtsize=${this.props.textSize}`;
        }

        return <img src={url} />;
    }

ended up as

    render() {
        let url = `https://placeholdit.imgix.net/~text?w=${this.props.width}&h=${this.props.height}`;

        if (this.props.text != null) {
            url += `&txt=${this
                .props
                .text
                .replace(/\s/, "+")}`;
            url += `&txtsize=${this.props.textSize}`;
        }

        return <img src={url}/>;
    }

Noteworthy might be that it formatted the first chained call while it left the second one alone (perhaps cause the first one was within an ES6 substitute string syntax or the second one didn't contain a method call but only attributes gettting accessed (just a blind guess))?
Another example would be

    return <footer className="footer">
      <div className="container">
        <p className="text text-muted">
          Made with  from around the world. <br />
          © {date.getFullYear()} - All rights reserved.
        </p>
      </div>
    </footer>;

which ended up as:

    render() {
        const date = new Date();

        return <footer className="footer">
            <div className="container">
                <p className="text text-muted">
                    Made with  from around the world.
                    <br/>
                    © {date.getFullYear()}
                    - All rights reserved.
                </p>
            </div>
        </footer>;
    }

Also:

        this.props.index.forEach((countries, countryId) => {
            countries.forEach((location, locationId) => {
                locations.push(this.props.index.get(countryId).get(locationId));
            });
        });

ended up as

        this
            .props
            .index
            .forEach((countries, countryId) => {
                countries.forEach((location, locationId) => {
                    locations.push(this.props.index.get(countryId).get(locationId));
                });
            });

It would be nice (don't know about feasibility) to:

  1. have an option to not touch any chained calls which are on the same line (perhaps that would make @ShMcK happy as well)
  2. have an option to not put the first call on a new line but only the first chained one (not sure if anyone puts everything on new lines, my preferred way is to only put the first chained call on a new line).
  3. it got completely confused by the following component:
import * as React from "react";
import { Link } from "react-router";

class HeaderComponent extends React.Component<any, any> {
    render() {
        return <nav className="navbar navbar-inverse navbar-fixed-top">
            <div className="container-fluid">
                <div className="navbar-header">
                    <button type="button" className="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
                        <span className="sr-only">Toggle navigation</span>
                        <span className="icon-bar"></span>
                        <span className="icon-bar"></span>
                        <span className="icon-bar"></span>
                    </button>
                    <a className="navbar-brand" href="#">Project name</a>
                </div>
                <div id="navbar" className="collapse navbar-collapse">
                    <ul className="nav navbar-nav navbar-right">
                        <li className="active"><Link to="/">Locations</Link></li>
                        <li><a href="#about">About</a></li>
                        <li><a href="#contact">Contact</a></li>
                    </ul>
                </div>
            </div>
        </nav>;
    }
}

export default HeaderComponent;

I'm not sure why, cause those imports are used in other files as well, where they are properly handled, but perhaps it helps you to identify the underlying issue.

That said, would it be possible to work together with @Glavin001 and add tsx support to atom-beautify as opt-in?

If that is combined with a link to some known issues list you hopefully wont get too many duplicates while getting more examples where things don't work as intended. WDYT?

@prettydiff
Copy link
Collaborator

It is only breaking chained properties if one of the properties is a method. I could probably be more uniform about that so that I am always breaking property chains. There is an Atom Beautify to limit this behavior "Break chain methods".

If TSX support is fine then somebody just needs to submit an Atom Beautify pull request to add support for the TSX grammar as it would get the same treatment as current JSX support. I am not a TSX user myself so I have just been needing validation and change recommendations.

@Glavin001
Copy link
Owner

If TSX support is fine then somebody just needs to submit an Atom Beautify pull request to add support for the TSX grammar as it would get the same treatment as current JSX support.

For those interested, see How to add a Language in the documentation found at https://github.com/Glavin001/atom-beautify/blob/master/docs/add-languages-and-beautifiers.md#how-to-add-a-language

Please let me know if you have any questions. While I am unable to allocate time to implementing these new languages into Atom Beautify, I always want to try and encourage collaborators and will do my best to find time to respond with helpful answers. Thank you.

@serut
Copy link

serut commented Jul 27, 2016

+1 for tsx support

@eunleem
Copy link

eunleem commented Oct 14, 2016

+1 for tsx support

@prettydiff
Copy link
Collaborator

TypeScript and TSX supported in the current version of Pretty Diff. An earlier comment mentioned a problem in the way Pretty Diff breaks methods onto lines. I plan to rewrite this logic soon: prettydiff/prettydiff#374

Before you can access the latest version of Pretty Diff in Atom Beautify I need to solve a distribution problem, which I am working at https://github.com/prettydiff/biddle and plan to move from Alpha to Beta version later today.

@davidjcastner
Copy link

+1 for tsx support

@Glavin001
Copy link
Owner

Everyone please stop bumping this and instead add your 👍 on the Issue post at the top. I only see ":+1: 2" at the top however here at the bottom are 5+ more 👍 which would not be seen. I also want tsx support.

What we need to make this happen is someone from the community to submit a Pull Request implementing this functionality. It may be as simple as upgrading Pretty Diff and adding .tsx language support in Atom-Beautify. @prettydiff may be able to help. Thanks in advance!

@prettydiff
Copy link
Collaborator

Keeping everybody informed I have fixed biddle. Check it out now - https://github.com/prettydiff/biddle

I will republish Pretty Diff using biddle in production tomorrow. I am not ready to give it to Atom Beautify just yet though. I actually need users to play around with this thing for a bit so that I can get a sense of where the pain points are and just how bad it works for fails for other people. I wrote a getting started document at https://github.com/prettydiff/biddle/blob/master/documentation/gettingstarted.md

The documentation might be easier to read from the command line though as I included a markdown to CLI parser in biddle. The number at the end defines the word wrap limit:

node biddle markdown documentation/gettingstarted.md 80

@godart
Copy link

godart commented Feb 24, 2017

This needs prettydiff 2.1.X which will be provided with #881

@wbhob
Copy link

wbhob commented Jun 4, 2017

What's the status on this issue @Glavin001 @prettydiff? Not much info since January...

@Glavin001
Copy link
Owner

@wbhob Waiting on @prettydiff and #881 . Please subscribe to #881 for progress updates.

@Glavin001
Copy link
Owner

Actually the TypeScript Formatter may work already:


This feature request looks to be closed: vvakame/typescript-formatter#18

@wbhob (and others), if you want to get this support official then we need to verify is the TypeScript Formatter works.

Step 1. Create a .tsx file in Atom
Step 2. Change grammar from TypeScriptReact to TypeScript. We need to do this because the TypeScript language does not support TypeScriptReact grammar or file extension .tsx.
Step 3. Beautify the .tsx file as raw TypeScript using the TypeScript Formatter.

If all goes well and the .tsx file is correctly beautified then all we need is the following:

Let me know if you have any questions. Looking forward to seeing a Pull Request!

@wbhob
Copy link

wbhob commented Jun 5, 2017

Closing tags act up a little bit in tsx...

@Glavin001
Copy link
Owner

In that case please feel free to create an Issue over at https://github.com/vvakame/typescript-formatter and see if they can help support your use case. Once either Pretty Diff or TypeScript Formatter works as expected on their own, we can add support to Atom-Beautify.

@briefguo
Copy link

+1

1 similar comment
@bakowroc
Copy link

+1

@prettydiff
Copy link
Collaborator

If anybody wants TSX support right now then help get this pull request to pass a build: #1750

@prettydiff
Copy link
Collaborator

Pretty Diff with TypeScript and TSX support are now in Atom Beautify and will be available once Atom Beautify is published to APM.

@traviswimer
Copy link

@prettydiff There have been a few releases since then, but I still get a message that TSX is not supported. Any idea when this feature will actually be released?

@traviswimer
Copy link

So its now been a few months. Does anyone know why TSX support isn't available yet? It isn't just that I need to change something in the settings, right?

@stevenzeck stevenzeck reopened this Jan 30, 2018
@stevenzeck
Copy link
Contributor

@traviswimer it's not quite that simple, but can be done, I'll see if I can get around to it soon.

@traviswimer
Copy link

@szeck87 Based on what @prettydiff said when closing the issue, I thought the work was already done for this, and it was just a matter of waiting for a new release. Is that not the case?

@stevenzeck
Copy link
Contributor

I don't see that it's supported in Atom Beautify. @prettydiff can you elaborate from your last comment? I don't see it anywhere here: https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/prettydiff.coffee#L46-L70

@stevenzeck
Copy link
Contributor

I added this to my pull request to update typescript-formatter. @Glavin001 can you please review?

@Glavin001
Copy link
Owner

Merged #2021
Coming in next release.

@Glavin001
Copy link
Owner

Published to v0.32.0

@Glavin001 Glavin001 added this to the v0.32.0 milestone Mar 2, 2018
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