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

[WIP] Fix trailing newline and JSX formatting #19

Closed
wants to merge 8 commits into from
Closed

[WIP] Fix trailing newline and JSX formatting #19

wants to merge 8 commits into from

Conversation

myitcv
Copy link
Contributor

@myitcv myitcv commented Aug 3, 2015

@vvakame - very much a work in progress but these changes work for me.

  • Using a nightly build of TypeScript until v1.6.0 is released (this is a dependency)
  • Using the latest version of type definitions from DefinitelyTyped
  • Write messages to process.(stdout|stderr)

I've no idea how the npm-shrinkwrap.json file has so many changes....new to all this npm stuff

@vvakame vvakame self-assigned this Aug 4, 2015
@vvakame
Copy link
Owner

vvakame commented Aug 4, 2015

@myitcv thanks you for your contributions!
I make a new commit. b9196e9

@vvakame
Copy link
Owner

vvakame commented Aug 4, 2015

please make minimal set pull request for topic.
I use typescript@latest for the moment.

@myitcv
Copy link
Contributor Author

myitcv commented Aug 4, 2015

@vvakame - thanks for the commit. However for everything to work for .jsx we also need to depend on typescript@next. Reason being this change is not in v1.5.3

So maybe the best thing is to create a branch of this project which is dedicated to v1.6.0 of TypeScript and reference typescript@next whilst we develop that?

@vvakame
Copy link
Owner

vvakame commented Aug 4, 2015

@myitcv tsfmt uses peer typescript compiler likes grunt-ts (https://github.com/TypeStrong/grunt-ts#compiler
If you want to use typescript@next, use it in your project. tsfmt will find it.
https://gist.github.com/vvakame/9a6459464258c7de63f9#file-package-json-L6
So it does no good?

@myitcv
Copy link
Contributor Author

myitcv commented Aug 4, 2015

Do I need to do anything in particular to set the peer typescript version, other than specify the version in package.json?

Here's a demo of the problem I'm seeing:

https://github.com/myitcv/tsfmt_funniness

$ npm install
...
$ ./node_modules/.bin/tsc --version
message TS6029: Version 1.6.0-dev.20150804
$ ./node_modules/.bin/tsc --jsx preserve input.tsx
$ cat input.jsx
function test() {
    return <span>This is a test</span>;
}
$ ./node_modules/.bin/tsfmt input.tsx
function test(): any {
    return <span>This is a test< /span>;
}

Notice the bad formatting of < /span> again.

tsfmt is using typescript from node_modules/typescript-formatter/node_modules/typescript/bin/typescript.js... which is not the version I have installed:

$ sed -n 6538,6548p node_modules/typescript-formatter/node_modules/typescript/bin/typescript.js
        function createSourceFile(fileName, languageVersion) {
            sourceFile = createNode(228 /* SourceFile */, 0);
            sourceFile.pos = 0;
            sourceFile.end = sourceText.length;
            sourceFile.text = sourceText;
            sourceFile.parseDiagnostics = [];
            sourceFile.bindDiagnostics = [];
            sourceFile.languageVersion = languageVersion;
            sourceFile.fileName = ts.normalizePath(fileName);
            sourceFile.flags = ts.fileExtensionIs(sourceFile.fileName, ".d.ts") ? 2048 /* DeclarationFile */ : 0;
        }

Does that make sense?
Am I doing something wrong?

@vvakame
Copy link
Owner

vvakame commented Aug 4, 2015

oh, I understand... npm can't flatten dependencies when typescript@next uses.
I consider what to do.

@myitcv
Copy link
Contributor Author

myitcv commented Sep 22, 2015

Closing in favour of #24

@myitcv myitcv closed this Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants