Add support for incremental compilation for typescript test

[abrenneke]

This makes a few small changes so that the typescript test supports incremental builds. Build info is saved in a .betterer.tsbuildinfo file next to the config file passed in to the test.

For me, betterer run time gets decreased from 20s to 8s, as long as it's run once before (fresh builds still take the full 20s). This is still far behind the tsc --build time of 9s fresh and 1.5s incremental. I haven't been able to figure out why that is so fast (it uses a different method, ts.createSolutionBuilderHost instead of ts.createIncrementalCompilerHost, but it's weird that the latter would be so much slower.

Unfortunately, this is still far too slow to run in lint-staged for us, and it's also pretty slow with the VS Code extension (who wants to wait 8 seconds to get squiggly lines), so there's room for improvement still. The extension should probably spawn a tsserver with the changed settings and communicate with that to get feedback? That gets pretty complicated though. Maybe #629 will enable better performance in lint-staged by only running on changed files? TS compilation is still the big roadblock here, though.

I think this requires either incremental or composite enabled in the source tsconfig to actually work incrementally. That seems fine to me.

I suppose this would break for typescript versions before 3.4, is that a problem?

[abrenneke]

Doesn't support the return value of createIncrementalProgram so the individual diagnostics are just spread into the big array instead

[phenomnomnominal]

Oh thank you so much for this! I've been fiddling with a PR for specifying file paths (for the lint-staged use-case). Do you think that would cover it or would you like the incremental building too?

Edit: Oops didn't see that you'd mentioned 629, but yes that is the intention.

[abrenneke]

Yeah, I wish that PR would help with the problem, but there isn't really a way to diagnostic-check only a single file in typescript without using tsserver and a file watch (from what I can tell).

If incremental here could be improved to the same as tsc --build (1.5s) that would be great for a lint-staged job, but I'm not sure how to speed it up right now.

Looking forward to that PR though for eslint checking reasons though! We have a bunch of rules we'd like to phase in. vscode support for this project is probably the biggest draw, so that we can see errors, but they don't affect the actual build.

[abrenneke]

(also, is the build failure related to my changes here? Seems strange that the extension would fail)

[phenomnomnominal]

The failure is definitely not your issue, that test is just flakey af at the moment (on my list 😅).

Is the project your working with open source? I have a few things I'd like to try out like if you can reuse an existing .tsbuildinfo file from a non-betterer build.

Also I'm thinking that it might be better to opt in to incremental mode rather than always setting it as it might not be compatible with all builds. Perhaps if any of the relevant flags are set on the input config or the read file?

[phenomnomnominal]

Also would you mind updating to the latest version (should be 4.2.0 for most packages) and try it out with lint-staged?

[abrenneke]

Is the project your working with open source? I have a few things I'd like to try out like if you can reuse an existing .tsbuildinfo file from a non-betterer build.

Closed source, unfortunately. I thought about reusing a tsbuild file, but since a betterer config can change anything in the tsconfig, I was worried they'd clobber each other and cause tons of problems.

Also I'm thinking that it might be better to opt in to incremental mode rather than always setting it as it might not be compatible with all builds. Perhaps if any of the relevant flags are set on the input config or the read file?

Yeah I suppose that makes sense, but I think it won't actually compile incrementally unless the tsconfig has "incremental": true or "composite": true

Also would you mind updating to the latest version (should be 4.2.0 for most packages) and try it out with lint-staged?

Sure as soon as I get the time! 😉

[abrenneke]

like if you can reuse an existing .tsbuildinfo file from a non-betterer build.

I've been experimenting with that and it seems like you can reuse the build info files, assuming you haven't changed anything that could change the signature of the compiled files in your compilerOptions overrides. It seems safer for that to be opt-in though, wouldn't want betterer clobbering your tsbuildinfo with its own build info every time you ran it.

Also would you mind updating to the latest version (should be 4.2.0 for most packages) and try it out with lint-staged?

Definitely better, and works great with eslint. The best we can to for typescript though is to bail out early if the file(s) in question aren't part of the build at all. Otherwise it has to build the whole TS project to figure out if there are any problems with the one file (and that's ~10 seconds)

Since we're in a monorepo, the betterer config file (in the workspace root) ended up looking something like this:

export default {
  'package-1 ts strict mode': typescript(
    './packages/package-1/tsconfig.json',
    {
      strict: true,
    },
    { reuseTsBuildInfo: true },
  ),
  '[fast] package-1 eslint rules': eslint(etc).include(
    './packages/package-1/src/**/*.ts',
  ),
  'package-2 ts strict mode': typescript(
    './packages/package-2/tsconfig.json',
    {
      strict: true,
    },
    { reuseTsBuildInfo: true },
  ),
  '[fast] package-2 eslint rules': eslint(etc).include(
    './packages/package-2/src/**/*.ts',
  ),
};

[fast] can be used in a filter in lint-staged for fast checks suitable for lint-staged, and the whole thing can be run as part of CI.

Still not sure what's causing the 10 seconds to run an incremental program each time.

[phenomnomnominal]

So, is there any reason to not use createIncrementalCompilerHost and createIncrementalProgram all the time? Would it make sense to just add docs saying to use incremental + composite + whatever tsBuildInfoFile you want?

[abrenneke]

So, is there any reason to not use createIncrementalCompilerHost and createIncrementalProgram all the time? Would it make sense to just add docs saying to use incremental + composite + whatever tsBuildInfoFile you want?

That's my impression right now. Been experimenting. This is the mess I have right now:

/**
 * Same as betterer/typescript. but adds support for incremental compliation.
 * See https://github.com/phenomnomnominal/betterer/pull/650
 */
export function typescript(
  configFilePath: string,
  extraCompilerOptions: ts.CompilerOptions,
  options?: Partial<TypescriptOptions>,
): BettererFileTest {
  if (!configFilePath) {
    throw new BettererError(
      "for `@betterer/typescript` to work, you need to provide the path to a tsconfig.json file, e.g. `'./tsconfig.json'`. ❌",
    );
  }
  if (!extraCompilerOptions) {
    throw new BettererError(
      'for `@betterer/typescript` to work, you need to provide compiler options, e.g. `{ strict: true }`. ❌',
    );
  }

  const resolver = new BettererFileResolver();
  const absPath = resolver.resolve(configFilePath);

  if (!existsSync(absPath)) {
    throw new BettererError(`tsconfig file does not exist: ${absPath}`);
  }

  return new BettererFileTest(resolver, async (filePaths, fileTestResult) => {
    const { config } = ts.readConfigFile(absPath, ts.sys.readFile.bind(ts.sys)) as TypeScriptReadConfigResult;
    const { compilerOptions } = config;
    const basePath = path.dirname(absPath);

    const originalTsBuildInfo = compilerOptions.tsBuildInfoFile ?? `${path.basename(absPath, '.json')}.tsbuildinfo`;

    const fullCompilerOptions: ts.CompilerOptions = {
      ...compilerOptions,
      tsBuildInfoFile: options?.reuseTsBuildInfo ? originalTsBuildInfo : path.join(basePath, '.betterer.tsbuildinfo'),
      ...extraCompilerOptions,
      // noEmit: true,
    };

    config.compilerOptions = fullCompilerOptions;

    const tempHost = ts.createCompilerHost(fullCompilerOptions);
    const configHost: ts.ParseConfigHost = {
      ...tempHost,
      readDirectory: ts.sys.readDirectory.bind(ts.sys),
      useCaseSensitiveFileNames: tempHost.useCaseSensitiveFileNames(),
    };
    const fullConfig = ts.parseJsonConfigFileContent(config, configHost, basePath);
    const host = ts.createIncrementalCompilerHost(fullConfig.options);
    const rootNames = new Set(await resolver.validate(fullConfig.fileNames));

    if (filePaths.length > 0 && !filePaths.some((p) => rootNames.has(p))) {
      return;
    }

    const oldProgram = ts.readBuilderProgram(fullConfig.options, host);

    const program = oldProgram
      ? ts.createEmitAndSemanticDiagnosticsBuilderProgram(Array.from(rootNames), fullConfig.options, host, oldProgram)
      : ts.createIncrementalProgram({
          options: fullConfig.options,
          rootNames: Array.from(rootNames),
          host,
        });

    const { diagnostics } = program.emit();

    const allDiagnostics = ts.sortAndDeduplicateDiagnostics([
      ...diagnostics,
      ...program.getConfigFileParsingDiagnostics(),
      ...program.getOptionsDiagnostics(),
      ...program.getSyntacticDiagnostics(),
      ...program.getGlobalDiagnostics(),
      ...program.getSemanticDiagnostics(),
    ]);

    allDiagnostics
      .filter(({ file, start, length }) => file && start != null && length != null)
      .forEach((diagnostic) => {
        const { start, length } = diagnostic as ts.DiagnosticWithLocation;
        const source = (diagnostic as ts.DiagnosticWithLocation).file;
        const { fileName } = source;
        const file = fileTestResult.addFile(fileName, source.getFullText());
        const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, NEW_LINE);
        file.addIssue(start, start + length, message);
      });
  });
}

[phenomnomnominal]

Just merged this, which should mean you can do something like...

import { typescriptΔ } from '@betterer/typescript';

export default {
  'typescript': typescriptΔ('./tsconfig.json', {
    incremental: true,
    tsBuildInfoFile: './.betterer.tsbuildinfo'
  }).include('./src/**/*.ts')
};

Want to try it out and see if its better? I'm publishing right now. I'll swap typescript for the new typescriptΔ implementation in V5 if it's better! I borrowed most of what you did above and combined it with my most recent caching stuff, so thank you so much!

[abrenneke]

Awesome!! Will check it out soon!