Use single quote literals in output for tslint

[darcyparker]

@bcherny - I revised this pull request based on last week's feedback to add tests. It is also rebased on top of the latest master.

I am not sure if the tests are where you'd like them. Some of the existing cases already test the single quotes. But I put some explicit ones here: 8181b0c#diff-632988bf801a8c538f18e7c4e222d9caR7

[darcyparker]

When I run npm test on this branch, all tests pass 26 passed. I wonder if there is a need of an npm clean script. (Sometimes I found I had to remove dist_test manually....) So as an experiment, I updated test script in package.json.

Unfortunately this didn't help. Then I noticed some failed tests in the commit history here: https://github.com/bcherny/json-schema-to-typescript/commits/master It looks like this might be circle specific issue because I don't have any failed tests on my end.

[darcyparker]

notice circleCI is expecting many Object{enum:#Array#,tsEnumNames:#Array#} Something strange is happening here.

[darcyparker]

I found problem... it has nothing to do with the `#Arrray# mentioned in last comment. I just misread the logging.

The problem is that there is a indent whitespace issue . I observed this problem in some of my JSONSchema to TS interface compile output elsewhere this morning too.

Pull up these two strings from circleCI and scroll towards the end of the line. (or put it in your fav diff tool). Notice some lines aren't indented for some reason...

export enum NamedIntegerEnum {\n  One = 1,\n  Two = 2,\n  Three = 3\n}\nexport enum ImpliedNamedIntegerEnum {\n  Four = 4,\n  Five = 5,\n  Six = 6\n}\nexport interface Enum {\n  stringEnum: 'a' | 'b' | 'c';\n  impliedStringEnum: 'a' | 'b' | 'c';\n  literalWithSingleQuote: 'Some \\'string\\' with inner single quote' | 'b' | 'c';\n  booleanEnum: true;\n  impliedBooleanEnum: true;\n  integerEnum: -1 | 0 | 1;\n  impliedIntegerEnum: -1 | 0 | 1;\n  numberEnum?: -1.1 | 0 | 1.2;\n  namedIntegerEnum?: NamedIntegerEnum;\n  impliedNamedIntegerEnum: ImpliedNamedIntegerEnum;\n  impliedHeterogeneousEnum?: -20.1 | null | 'foo' | false;\n}
export enum NamedIntegerEnum {\n  One = 1,\n  Two = 2,\n  Three = 3\n}\nexport enum ImpliedNamedIntegerEnum {\n  Four = 4,\n  Five = 5,\n  Six = 6\n}\nexport interface Enum {\n  stringEnum: 'a' | 'b' | 'c';\n  impliedStringEnum: 'a' | 'b' | 'c';\n  literalWithSingleQuote: 'Some \\'string\\' with inner single quote' | 'b' | 'c';\n  booleanEnum: true;\n  impliedBooleanEnum: true;\n  integerEnum: -1 | 0 | 1;\nimpliedIntegerEnum: -1 | 0 | 1;\nnumberEnum ?: -1.1 | 0 | 1.2;\nnamedIntegerEnum ?: NamedIntegerEnum;\nimpliedNamedIntegerEnum: ImpliedNamedIntegerEnum;\nimpliedHeterogeneousEnum ?: -20.1 | null | 'foo' | false;\n}

Now the question is... why does circleCI have different results than my local machine (and presumably yours too because I doubt you pushed your commits to master if the tests failed like circleCI). And why does some of my output for different JSON schema sometimes miss the indent.

[bcherny]

@darcyparker I'm aware of the whitespace issue, and am looking at it. My hunch is it's related to unix vs windows line endings, but I need to look at it more closely.

Will review + merge this PR once the build issue is resolved.

[darcyparker]

Great - glad you are on it.

[bcherny]

And why does some of my output for different JSON schema sometimes miss the indent.

This fails for you locally sometimes? Are you running OSX?

[darcyparker]

Tests always pass. But I have seen indent issues today (not last week) for some json schema outside this repo. I am on Debian Linux.

[bcherny]

@darcyparker Ready to go on my end

[darcyparker]

Thanks - BTW: updated comments look nice.
This is rebased and ready for review.

[bcherny]

@darcyparker I see what you're going for here, but I think it could be overcomplicating things. JSON.stringify already handles escaping double quotes for us in a reliable way. I'd prefer to keep outputting double quotes rather than implementing a custom stringifier.

What do you think?

[darcyparker]

@bcherny - I understand your concern. You're right that JSON.stringify is probably best.

It's unexpected that the output style of the compiler is different than the project (semicolons and double quotes...) But I agree it is best to use JSON.stringify because it is correct and simpler.

As I reflect on this, people are going to have different preferences for tslint (I prefer semicolons and single quotes for example) and the typescript definition output from the compiler will never satisfy everyone's preferences. So it is probably best to advise people to use a formatter such as https://github.com/vvakame/typescript-formatter to tune the output. (tsfmt doesn't seem to support setting single vs double quotes because typescript language service does not)

[darcyparker]

I will close this...

Do you want any of the following commits in a new pull request? Clean dist_tests before building tests, disable some tslint rules in test schemas, fix tslint no-consecutive-blank-lines, fix tslint missing whitespace, fix tslint unnecessary semicolon

They are pretty minor I know.... so maybe you'll get them in future update.

[bcherny]

@darcyparker Sure - feel free to put all except disable some tslint rules in test schemas in a single PR.

[bcherny]

It's unexpected that the output style of the compiler is different than the project (semicolons and double quotes...)

I agree.

So it is probably best to advise people to use a formatter

Also agreed! I removed the auto formatter in 1aa7317 because it was causing those occasional indentation issues on CI. We can also use tslint directly or maybe tslint-fix. In any case, this is pretty low priority. Filed #45 to track it.