From eaeb743e9cd3f47cdbf4242a1e0acf09b2a7c2b2 Mon Sep 17 00:00:00 2001 From: Allison Karlitskaya Date: Sat, 29 Jun 2024 17:47:59 +0200 Subject: [PATCH] test/static-code: tweak Typescript config again Having `checkLib: false` in `tsconfig.conf` is a bad idea: it means that if we want to check `.d.ts` files, we need a separate invocation of the typescript compiler to do that. It also means that `.d.ts` files won't be checked in your editor by default. Our workaround for this in `test/static-code` only checks `cockpit.d.ts`, and we'll soon have other declaration files. We've been using `checkLib: false` as a crutch to avoid reporting errors against `node_modules/`. The relevant TypeScript issue is still unaddressed: https://github.com/microsoft/TypeScript/issues/30511 Modify `test/static-code` to only report errors if errors were found on something outside of node_modules, but in that case, report all errors. We do this with two invocations of `tsc`, to keep things simple, but we'll only do the second invocation if we're reporting errors. To keep things speedy, we turn on "incremental" mode (which writes a cache file to speed things up). `.gitignore` that. It now takes only ~2s to do an incremental check in the "no errors" case. By default, `tsc` outputs a very nice format that shows the exact content of errors, but this doesn't work by default in `test/static-code` because we capture the output before displaying it. Detect if we're running on a TTY (or if `FORCE_COLOR=1` as from our workflows) and enable the "pretty" output in that case. --- .gitignore | 1 + test/static-code | 16 +++++++++------- tsconfig.json | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 2cf7d3fcba..9fee36a00a 100644 --- a/.gitignore +++ b/.gitignore @@ -62,6 +62,7 @@ Makefile.in /test_rsa_key /tmp-dist /tmp/ +/tsconfig.tsbuildinfo /version.m4 /wsinstance-start diff --git a/test/static-code b/test/static-code index 1e12a5a5a9..318b340cd0 100755 --- a/test/static-code +++ b/test/static-code @@ -69,13 +69,11 @@ if [ "${WITH_PARTIAL_TREE:-0}" = 0 ]; then test_typescript() { test -x node_modules/.bin/tsc -a -x /usr/bin/node || skip 'no tsc' - # https://github.com/microsoft/TypeScript/issues/30511 - # We can't tell tsc to ignore the .d.ts in node_modules/ and check our - # own cockpit.d.ts, so we do two separate invocations as a workaround: - if [ -n "$(git ls-files pkg/lib)" ]; then - node_modules/.bin/tsc --typeRoots /dev/null --strict pkg/lib/cockpit.d.ts - fi - node_modules/.bin/tsc --checkJs false --skipLibCheck + # Only display tsc output if it contains errors about something other than node_modules/ + # https://github.com/microsoft/TypeScript/issues/30511 + if node_modules/.bin/tsc --checkJs false --pretty false | grep -Eqv '^(node_modules| )'; then + node_modules/.bin/tsc ${FORCE_COLOR:+--pretty true} --checkJs false + fi } fi @@ -162,6 +160,10 @@ main() { exit 0 fi + if test -t 1; then + FORCE_COLOR=1 + fi + exit_status=0 counter=0 diff --git a/tsconfig.json b/tsconfig.json index 5207dbd967..08359e313d 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,7 @@ "baseUrl": "./pkg/lib", "esModuleInterop": true, "exactOptionalPropertyTypes": true, + "incremental": true, "jsx": "react", "lib": [ "dom", @@ -21,7 +22,6 @@ ], "moduleResolution": "node", "noEmit": true, // we only use `tsc` for type checking - "skipLibCheck": true, // don't check node_modules "strict": true, "target": "es2020", },