As we convert libraries from gulp to gts, the only thing I'm really missing is npm run watch. I'd like to run that command, have it automatically watch for changes in *.ts* files or package.json, and react by running the 'compileandtest` commands. I could see a syntax like this:

$ gts watch

By default, it would monitor the sources directory for changes, and run npm run compile on change. We wouldn't let users change it. After compilation, it would run npm test. On test failure, we would have to be careful to not let the process stop.

If we want to get fancy - we could support a syntax that runs the start process instead of tests on file change. This would be useful for web applications where you're more concerned with live development:

$ gts watch --start

I'm a little conflicted, because this goes beyond the scope of what we're building - but it is a useful thing I just want to have out of the box with most TypeScript projects.

Thoughts?

From our internal TypeScript style guide:

Do not use trailing or leading underscores for private properties or methods.

Would be great to detect this and give an error. Not sure if that's the kind of thing we want to auto-fix or not.

Opening issues for the 1.0 milestone. TypeScript 2.8 seems like a good target: https://twitter.com/typescriptlang/status/974394621601095680. This needs to happen in a semver major, so 1.0 seems like the right time to do this.

With the latest version gts, if I use a globally installed one to generate a new project with gts init, the generated tsconfig.json looks like this.

{
  "extends": "./../.nvm/versions/node/v9.3.0/lib/node_modules/gts/tsconfig-google.json",
  "compilerOptions": {
    "rootDir": ".",
    "outDir": "build"
  },
  "include": [
    "src/*.ts",
    "src/**/*.ts",
    "test/*.ts",
    "test/**/*.ts"
  ],
  "exclude": [
    "node_modules"
  ]
}

See the first line. It should use a locally installed gts path:

{
  "extends": "./node_modules/gts/tsconfig-google.json",
  ...
}

I'm not sure since when this test started to fail.

test.ts is presently using a staging directory. Switch it to use tmp.

See below comment

On my mac, gts check exits with code 0 before checking all files here. On linux this doesn't happen, and I get my clang-format error reports.

I combed gts dependencies for process.exit calls but couldn't find any. I believe unresolved Promises are somehow being ignored, and the event loops drains fully. Unsure about this...

Running gts check on the google-api-nodejs-client can take kind of a long time (minutes). To improve performance, would it be possible to do some sort of incremental build, where we only run the tool on files that have changed?

That test is a system-test. Ideally it would run after the unit test, if possible, but at the least it should be in a separate directory to make it easy to glob just the unit tests.

Opening issues for the 1.0 milestone. Node is going to go EOL by end of April, so let's drop support for it. This needs to happen in a semver major, so 1.0 is a good target.

• Update engines.node field in package.json.
• Change default target to ES2016. Node 6 has almost everything in ES2016.
• Enable --esModuleInterop (already present in next branch).

For example:

  "scripts": {
    "lint": "tslint -c ../../.npm/_npx/30782/lib/node_modules/google-ts-style/tslint.json -p . -t codeFrame --type-check"
  },

Note the path to a temporary npx folder. The expectation was to use a relative path to a local ./node_modules/ installation of google-ts-style.

When run using a non-local version of the module, we should bootstrap a package.json if necessary, install a local google-ts-style and then re-exec google-ts-style init.

@ofrobots brought this up in a discussion a few days ago on how we should go forward in following semver guidelines for gts, with the central issue that Google's TS style guide is a moving target, adopting new changes over time. To ensure that users of gts don't get caught unaware, it's important for us to consider how we can map changes to the style guide to semantic versioning levels in a way that makes sense.

If it can not be supported for this command, gts should throw an error if gts fix --dry-run is given.
If it can be supported, gts should output suggested changes.

@JustinBeckwith asked in #146:

May as well bump target in tsconfig.js to at least es2016. It's even possible es2017 works here 🥁

• There is only one feature in in ES2016 that is missing from Node 6 – exponentiation operator. There is a slight risk that transpiled code will not work with Node 6 if exponentiation operator is in use, but given that the use is rare, I think it makes sense to use es2016 as the default. Users that run into trouble can always downgrade the target in their tsconfig.json. There is significant benefit to majority of the users from es2016.
• es2017 has features that are only available in Node 8+ (e.g. async functions). I'm 👎 on switching to it.

We should have at least a few integration tests to make sure I'm not breaking anything 😂

I used npx gts init, and it generated a tsconfig.json for me (awesome). During that CR, @ofrobots mentioned that it should have a newline at the end. Fixing mine now, but should probably be fixed here :)

Also worth asking - if newlines at the end of a file are a thing we should be doing, shouldn't gts check for that and fix?

The specified path does not exist: '/users/dom/.npm/_npx/98679/lib/node_modules/gts/tsconfig-google.json'.

I am trying to use gts by doing an npm install -g gts. I got this library that's going to be a train wreck to upgrade, so I want to do it one piece at a time with a global tool. So I cd into my app dir, and run the check command, and this is the call stack:

beckwith-macbookpro:google-api-nodejs-client beckwith$ gts check
FatalError: Could not find config file at: /Users/beckwith/Code/google-api-nodejs-client/node_modules/gts/tslint.json
    at new FatalError (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/node_modules/tslint/lib/error.js:27:28)
    at findConfigurationPath (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/node_modules/tslint/lib/configuration.js:55:19)
    at Object.findConfiguration (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/node_modules/tslint/lib/configuration.js:41:22)
    at lint (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/lint.js:31:48)
    at /Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/cli.js:77:46
    at step (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/cli.js:33:23)
    at Object.next (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/cli.js:14:53)
    at /Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/cli.js:8:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/beckwith/.nvm/versions/node/v8.9.1/lib/node_modules/gts/build/src/cli.js:4:12)

#74 (review)

We right now have the following lint / format related rules:

• lint: runs linter, prints errors
• fix: runs linter, runs formatter, automatically fixes problems.

We are missing a verb to check format. Instead of adding a 'format' verb, I propose that we rename lint to check with the semantics that it would check both lint and format problems.

I had the following test case where durationMillis is unused. If I run gts fix or gts check I don't get any output.
Should this problem be caught?

it('should throw error when disabled', async () => {
      const durationMillis = 10 * 1000;
      const intervalBytes = 1024 * 512;
      const stackDepth = 32;
      const profiler = new HeapProfiler(intervalBytes, stackDepth);
      profiler.disable();
      try {
        const profile = await profiler.profile();
        assert.fail('Expected error to be thrown.');
      } catch (err) {
        assert.equal(err.message, 'Heap profiler is not enabled.');
      }
    });

If my tsconfig.json extends tsconfig.other.json with no additional parameters, gts clean will fail because it can't read build in tsconfig.json.

It should try to detect the extension chain and find the build field in tsconfig.other.json. Any other command that relies on a field in tsconfig.json might have the same issue.

We should add a command to do clean instead of inserting an rm -rf command into package.json.

fix and check can be used from a globally installed gts, but not check and clean.

This is useful for IDEs and tooling to recognize the intended clang-format settings for format-on-save behaviors etc.

The tsconfig file provided here includes the noUnusedParameters compiler option, which is useful. However, sometimes when implementing an interface, some parameters are not actually useful. In that case, if the argument name starts with an underscore, the compiler will not emit an error (see this documentation). Currently, if I try to do this, then I try to lint my files using the TSLint configuration provided here, it fails because the lint configuration does not allow any underscores in non-constant names.

Please fix this by adding the "allow-leading-underscore" option to the TSLint "variable-name" rule configuration.

$ npm run build

[email protected] build /Documents/my-module
npm run compile

[email protected] compile /Documents/pallas
tsc -p .

error TS18001: A path in an 'extends' option must be relative or rooted, but 'node_modules/google-ts-style/tsconfig-google.json' is not.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] compile: tsc -p .
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] compile script.

Consider #80. Even though the issue is closed there are some points the disposition and approach to that issue raise:

• The aim of this project (gts) seems to be to enable the Google style guide for anyone (Google, non-Google) who wants to use it.
• However, it is not the right venue to debate the style guide itself. We are basically mirroring what Google uses internally and cannot (should not) editorialize the style itself.
• This means that this repo isn't the best venue to debate the style guide.
• The style guide document itself isn't public (yet). This makes it hard for users to understand the context and rationale behind the style rules.

The question becomes, how do we deal with critiques of the style guide:

1. 'This is not the correct venue for debates on the style guide'. This is unfulfilling for external users.
2. Try to proxy concerns between internal and external venues. Somewhat better but still unfulfilling.
3. Over the longer term, make this repo host the externalized style guide & make this authoritative?

@JustinBeckwith @mprobst: Thoughts?

In a perfect world, we would have the same name for:

• The repository name
• The npm module name
• The binary name used in npm run

Today all three of these are different for us :( We have:

• google/ts-style for the GitHub name
• google-ts-style as the npm module name
• gts as the binary name

I think reducing this down into one name would make everything a lot easier to remember, and easier for new users figuring things out.

So to make things more concrete...

Let me throw out a few ideas.

1. We could rename the npm module to gts to keep it consistent with the binary. Optionally, we could consider renaming the GitHub repo.
2. We could pick a new name for all three, and try to move it all together. Some ideas (all available on npm):

• typefix
• @google/standardts
• blueshed
• `Any others welcomed....

Thoughts? Calling @ofrobots @MylesBorins @jmdobry ...

For code like this:

import {EventEmitter} from 'events';

interface MyData {
  foo: string;
}

class Foo extends EventEmitter {
  on(event: 'mydata', listener: (data: MyData) => void): this;
  on(event: string, listener: (...args: any[]) => void): this {
    return super.on(event, listener);
  }
}

, gts fix converts ...args: any[] to ...args: {}[], which makes the specialized on() method (for 'mydata') not compatible with the implementation and makes the build fail. I think this must be left as any[] because the super class (EventEmitter)'s type is any[].

That also brings another problem where the linter complains about {}[], saying that

Array type using 'T[]' is forbidden for non-simple types. Use'Array' instead.

Which forces me to write Array<{}>, which is harder to read than {}[] or any[]. It surprises me that it says {} is of a non-simple type.

Given that we have unit test now (#28), lets make sure they are covering all the code.

With #53 in place, let's get coverage to 100%.

Instead of requiring the peer dependency for clang, we should abstract the format behind gts format

Letting users control the typescript and tsconfig.json is absolutely the right call. For clang and tslint though... I wonder out loud if we shouldn't entirely abstract them behind a custom purpose binary. For example - today our npm scripts look like this:

  "scripts": {
    "build": "npm run compile",
    "clean": "rm -rf ./build/",
    "compile": "tsc -p . --rootDir . --outDir build/",
    "format": "clang-format -i -style=\"{Language: JavaScript, BasedOnStyle: Google, ColumnLimit: 80}\" src/*.ts test/*.ts",
    "lint": "tslint -c ../../.npm/_npx/30782/lib/node_modules/google-ts-style/tslint.json -p . -t codeFrame --type-check"
  },

If we provided a special purpose non-configurable binary, it would look like this:

"scripts": {
    "build": "npm run compile",
    "clean": "rm -rf ./build/",
    "compile": "tsc -p . --rootDir . --outDir build/",
    "format": "gts format src/*.ts test/*.ts",
    "lint": "gts lint"
},

I could also make the argument for gts clean, as I don't know if windows will support rm -rf that easily.

It would reduce dependencies down as well:

"devDependencies": {
    "google-ts-style": "latest",
    "typescript": "^2.4.1"
}

For context: googleapis/google-p12-pem#9 (comment)

+  "include": [
 +    "src/*.ts",
 +    "src/**/*.ts",
 +    "test/*.ts",
 +    "test/**/*.ts"
 +  ],
+  "exclude": [
 +    "node_modules"
 +  ]

I think you can leave 'include' out and it just compiles **/*.ts (obeying the exclude).
Alternatively, you can leave the exclude out. Specifying both is redundant I think.

IIRC, gts sets things up this way because of some trouble we some trouble with tsc ending up looking into node_modules despite the includes section. I can't remember the details at the moment however. We should revisit and document the reason for this.

$ npm run test

> [email protected] test /Users/ofrobots/src/google-ts-style
> npm run compile && nyc ava build/test/test*.js

> [email protected] compile /Users/ofrobots/src/google-ts-style
> tsc -p .
...
  lint › lint should return false on bad code

  /Users/ofrobots/src/google-ts-style/build/test/test-lint.js:17

   16:  */
   17: var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
   18:     return new (P || (P = Promise))(function (resolve, reject) {

Whereas the following is a lot better:

$ npm run compile && ava build/test/test-lint.js

> [email protected] compile /Users/ofrobots/src/google-ts-style
> tsc -p .
...
  lint should return false on bad code

  /Users/ofrobots/src/google-ts-style/test/test-lint.ts:83

   82:         const okay = lint.lint(OPTIONS);
   83:         t.is(okay, false);
   84:       });

So I've done this a few times now. I will run npx gts init, then immediately try doing a npm run compile. This always fails with an error like this:

error TS5058: The specified path does not exist:
 '/users/beckwith/code/testy/node_modules/gts/tsconfig-google.json'.

Then I remember to run npm install, and everything is fine. Is it possible to kick off an npm install after npx gts init modifies my package.json?

I set up my project so that my TypeScript configuration file is different from tsconfig.json. In addition, I have my own TSLint config file and I want to use it.

At the same time I want to use gts, but don't see the way to specify the paths to my TS and TSLint configs.

Can you please tell me, can I do it?

If this requires further work, I will be happy to write the PR (but I will need your advice).

We got integration tests mostly figured out. Need something we can use as a template for more unit tests moving forward.

Necessary for IDEs that have tslint integration. Would also enable extensibility of the tslint config where needed while supporting keeping up-to-date with base config.

See https://github.com/google/ts-style/pull/143/files#r178920063.

• We should default to build/src/index.js.
• We should include a types property in the generated package.json.

Despite installing typescript, the module requires typescript to be installed locally first. I think this wasn't the behavior in 0.1.2.

It seems like some cases, this might not work even with locally installed typescript.