Skip to content

Commit

Permalink
test/static-code: tweak Typescript config again
Browse files Browse the repository at this point in the history
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: microsoft/TypeScript#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.
  • Loading branch information
allisonkarlitskaya committed Jul 2, 2024
1 parent fe6b5f1 commit eaeb743
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 8 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Makefile.in
/test_rsa_key
/tmp-dist
/tmp/
/tsconfig.tsbuildinfo
/version.m4
/wsinstance-start

Expand Down
16 changes: 9 additions & 7 deletions test/static-code
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -162,6 +160,10 @@ main() {
exit 0
fi

if test -t 1; then
FORCE_COLOR=1
fi

exit_status=0
counter=0

Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
"baseUrl": "./pkg/lib",
"esModuleInterop": true,
"exactOptionalPropertyTypes": true,
"incremental": true,
"jsx": "react",
"lib": [
"dom",
"es2020"
],
"moduleResolution": "node",
"noEmit": true, // we only use `tsc` for type checking
"skipLibCheck": true, // don't check node_modules
"strict": true,
"target": "es2020",
},
Expand Down

0 comments on commit eaeb743

Please sign in to comment.