Skip to content

Commit 267db5d

Browse files
authored
chore(vitest): caching and add apperror checks (#327)
Adds a method of intermediate caching some passes to make reruns faster. Adds pass to check for AppError typing --------- Signed-off-by: Jurriaan Ruitenberg <[email protected]> Signed-off-by: jurriaanr <[email protected]> Co-authored-by: Jurriaan Ruitenberg <[email protected]>
1 parent a0c4b8a commit 267db5d

File tree

11 files changed

+219
-13
lines changed

11 files changed

+219
-13
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,4 @@ yarn-error.log*
4242
# Caches
4343
*.tsbuildinfo
4444
.cache
45+
packages/compas-convert/cache

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
},
2727
"workspaces": ["packages/*", "apps/*"],
2828
"volta": {
29-
"node": "22.9.0"
29+
"node": "22.11.0",
30+
"typescript": "5.7.2"
3031
}
3132
}

packages/compas-convert/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ rm -rf ../compas-convert-test && npx compas-convert ../some-local-test-project .
5656
- On `TS18046: 'e' is of type 'unknown'`
5757
- AND `.key` or `.info` is accessed
5858
- Implement like `not-nil-checks-in-test-flows`
59-
- [x] ~Convert JSDOC `function(number, string): string` to `(number, string) => string`~
59+
- [x] Convert JSDOC `function(number, string): string` to `(number, string) => string`
60+
(only 16 occurrences in codebase, maybe not worth it)
6061
- [x] Pass: query-builder types
6162
- Improve query-builder types by inferring the types based on the passed in builder.
6263

packages/compas-convert/src/context.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { Project } from "ts-morph";
88
export interface Context {
99
inputDirectory: string;
1010
outputDirectory: string;
11+
cacheDirectory: string;
1112
packageJson?: PartialTypedPackageJson;
1213
ts?: Project;
1314
pendingImports: Record<
@@ -29,10 +30,12 @@ export interface PartialTypedPackageJson {
2930
export function createEmptyContext(
3031
inputDirectory: string,
3132
outputDirectory: string,
33+
cacheDirectory: string,
3234
): Context {
3335
return {
3436
inputDirectory,
3537
outputDirectory,
38+
cacheDirectory,
3639

3740
pendingImports: {},
3841
};

packages/compas-convert/src/index.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#!/usr/bin/env node
22

33
import { existsSync } from "node:fs";
4+
import * as fs from "node:fs";
45
import path from "node:path";
56
import consola from "consola";
67
import { createEmptyContext } from "./context.js";
78
import type { GlobalPass, Pass } from "./pass.js";
89
import { addCommonImports } from "./passes/add-common-imports.js";
10+
import { isAppErrorInTestFiles } from "./passes/assert-is-app-error-in-test-files.js";
911
import { convertTestConfig } from "./passes/convert-test-config.js";
1012
import { convertTestFiles } from "./passes/convert-test-files.js";
1113
import { copyRename } from "./passes/copy-rename.js";
@@ -22,6 +24,7 @@ import { transformModuleJsDoc } from "./passes/transform-module-js-doc.js";
2224
import { fixTypesOfAllFunctions } from "./passes/types-of-all-functions.js";
2325
import { fixTypesOfLiveBindings } from "./passes/types-of-live-bindings.js";
2426
import { typescriptDiagnostics } from "./passes/typescript-save-and-build.js";
27+
import { Cache } from "./shared/cache.js";
2528
import { globOfAllTypeScriptFiles } from "./shared/project-files.js";
2629
import { isNil } from "./utils.js";
2730

@@ -42,7 +45,20 @@ if (isNil(outputDirectory) || existsSync(outputDirectory)) {
4245

4346
const resolvedInputDirectory = path.resolve(inputDirectory);
4447
const resolvedOutputDirectory = path.resolve(outputDirectory);
45-
const context = createEmptyContext(resolvedInputDirectory, resolvedOutputDirectory);
48+
49+
const cacheDirectory = path.resolve(
50+
`${path.dirname(import.meta.dirname)}/../cache/${path.basename(inputDirectory)}`,
51+
);
52+
53+
if (!existsSync(cacheDirectory)) {
54+
fs.mkdirSync(cacheDirectory, { recursive: true });
55+
}
56+
57+
const context = createEmptyContext(
58+
resolvedInputDirectory,
59+
resolvedOutputDirectory,
60+
cacheDirectory,
61+
);
4662

4763
const passes: Array<Pass> = [
4864
copyRename,
@@ -68,10 +84,18 @@ const passes: Array<Pass> = [
6884
// Re-init TS Morph. Since there is no clean way of refreshing diagnostics.
6985
initTsMorph,
7086
notNilChecksInTestFiles,
87+
isAppErrorInTestFiles,
7188

7289
typescriptDiagnostics,
7390
];
7491

92+
const cachablePasses = [
93+
"convertTestFiles",
94+
"notNilChecksInTestFiles",
95+
"isAppErrorInTestFiles",
96+
"finalizePendingImports",
97+
];
98+
7599
consola.start(`Converting ${path.relative(process.cwd(), resolvedInputDirectory)}`);
76100
for (const pass of passes) {
77101
consola.info(`Running ${pass.name}`);
@@ -91,8 +115,17 @@ for (const pass of passes) {
91115
}
92116

93117
try {
118+
const cache = new Cache(context, sourceFile);
119+
if (cachablePasses.includes(pass.name) && cache.useIfExists()) {
120+
continue;
121+
}
122+
94123
await pass(context, sourceFile);
95124
await sourceFile.save();
125+
126+
if (cachablePasses.includes(pass.name)) {
127+
cache.store();
128+
}
96129
} catch (e) {
97130
consola.debug({
98131
sourceFile: sourceFile.getFilePath(),

packages/compas-convert/src/passes/add-common-imports.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ export function addCommonImports(context: Context, sourceFile: SourceFile) {
2929
CONVERT_UTIL.assertNotNil,
3030
false,
3131
);
32+
addPendingImport(
33+
context,
34+
sourceFile,
35+
resolveRelativeImport(context, sourceFile, CONVERT_UTIL_PATH),
36+
CONVERT_UTIL.assertIsAppError,
37+
false,
38+
);
3239

3340
addPendingImport(context, sourceFile, "@compas/stdlib", "InsightEvent", true);
3441
addPendingImport(context, sourceFile, "@compas/stdlib", "Logger", true);
@@ -63,4 +70,14 @@ export function addCommonImports(context: Context, sourceFile: SourceFile) {
6370
addPendingImport(context, sourceFile, "axios", "AxiosInstance", true);
6471
addPendingImport(context, sourceFile, "axios", "AxiosRequestConfig", true);
6572
addPendingImport(context, sourceFile, "axios", "AxiosError", true);
73+
74+
if (sourceFile.getFilePath().endsWith(".test.ts")) {
75+
// re-add import for newTestEvent
76+
addPendingImport(context, sourceFile, "@compas/cli", "newTestEvent", false);
77+
addPendingImport(context, sourceFile, "@compas/stdlib", "newLogger", false);
78+
addPendingImport(context, sourceFile, "vitest", "expect", false);
79+
addPendingImport(context, sourceFile, "vitest", "beforeAll", false);
80+
addPendingImport(context, sourceFile, "vitest", "describe", false);
81+
addPendingImport(context, sourceFile, "vitest", "test", false);
82+
}
6683
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import type { SourceFile } from "ts-morph";
2+
import { Node, ts } from "ts-morph";
3+
import type { Context } from "../context.js";
4+
import SyntaxKind = ts.SyntaxKind;
5+
6+
/**
7+
* Add 'assertIsAppError' statements for type checking caught exception values in test files.
8+
*/
9+
export function isAppErrorInTestFiles(context: Context, sourceFile: SourceFile) {
10+
if (!sourceFile.getFilePath().endsWith(".test.ts")) {
11+
// We only run this for test files. All other files are runtime behavior which has to be
12+
// double-checked.
13+
return;
14+
}
15+
16+
let nextDiagnostic;
17+
// Keep track of the last position, so we only move forwards in the file, and don't get stuck on
18+
// an expression we can't fix.
19+
let lastPosition = 0;
20+
21+
while (
22+
(nextDiagnostic = getNextObjectOfTypeUnknownDiagnostic(sourceFile, lastPosition))
23+
) {
24+
// Move past the full error
25+
lastPosition = nextDiagnostic.getStart()! + nextDiagnostic.getLength()! + 1;
26+
27+
// For some reason, we can't find the position of the diagnostic.
28+
const expression = sourceFile.getDescendantAtPos(nextDiagnostic.getStart()!);
29+
if (!expression) {
30+
continue;
31+
}
32+
33+
// Find the wrapping statement. These errors are always inside an expression part of some
34+
// statement.
35+
const parentStatement =
36+
Node.isStatement(expression) ? expression : (
37+
expression.getParentWhile((_parent, node) => !Node.isStatement(node))
38+
);
39+
40+
if (!parentStatement) {
41+
continue;
42+
}
43+
44+
if (!isInCatchClause(parentStatement)) {
45+
continue;
46+
}
47+
48+
// The diagnostic text is not always helpful, so retrieve the expression from the file contents.
49+
// - "'e' is of type unknown'"
50+
const expressionMatch = sourceFile
51+
.getFullText()
52+
.slice(
53+
nextDiagnostic.getStart(),
54+
nextDiagnostic.getStart()! + nextDiagnostic.getLength()!,
55+
);
56+
57+
try {
58+
const keyOrInfoRegex = new RegExp(`${expressionMatch}\\.(?:key|info|status)\\b`);
59+
60+
if (
61+
parentStatement.getText().match(keyOrInfoRegex) &&
62+
isInCatchClause(parentStatement)
63+
) {
64+
const functionCallToInsert = `assertIsAppError(${expressionMatch});\n\n`;
65+
66+
// Assert is AppError does a type-narrowing assertion; meaning that it guarantees
67+
// TypeScript, that it would throw and thus prevents execution of the normal code-path.
68+
sourceFile.insertText(parentStatement.getStart(true), functionCallToInsert);
69+
70+
// Make sure to the offset of the inserted call as well, so we don't stay in an infinite
71+
// loop
72+
lastPosition += functionCallToInsert.length;
73+
}
74+
// eslint-disable-next-line unused-imports/no-unused-vars
75+
} catch (e) {
76+
return;
77+
}
78+
}
79+
}
80+
81+
/**
82+
* Get the next diagnostic which we can possible fix.
83+
* This way we rerun the diagnostics each time, getting an up-to-date view. Since one assertion can
84+
* fix multiple errors.
85+
*/
86+
function getNextObjectOfTypeUnknownDiagnostic(
87+
sourceFile: SourceFile,
88+
fromPosition: number,
89+
) {
90+
const errorCodes = {
91+
objectIsOfTypeUnknown: 18046,
92+
};
93+
94+
return sourceFile.getPreEmitDiagnostics().find((it) => {
95+
return (
96+
Object.values(errorCodes).includes(it.getCode()) &&
97+
(it.getStart() ?? 0) > fromPosition
98+
);
99+
});
100+
}
101+
102+
function isInCatchClause(node: Node) {
103+
const parent = node.getParentWhile(
104+
(parentNode) => !parentNode.isKind(SyntaxKind.CatchClause),
105+
);
106+
return parent !== null;
107+
}

packages/compas-convert/src/passes/convert-test-files.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type { CallExpression, SourceFile } from "ts-morph";
22
import { Node } from "ts-morph";
33
import { ts } from "ts-morph";
44
import type { Context } from "../context.js";
5-
import { addPendingImport } from "../shared/imports.js";
65
import SyntaxKind = ts.SyntaxKind;
76

87
/**
@@ -49,14 +48,6 @@ export function convertTestFiles(context: Context, sourceFile: SourceFile) {
4948
)
5049
?.remove();
5150

52-
// re-add import for newTestEvent
53-
addPendingImport(context, sourceFile, "@compas/cli", "newTestEvent", false);
54-
addPendingImport(context, sourceFile, "@compas/stdlib", "newLogger", false);
55-
addPendingImport(context, sourceFile, "vitest", "expect", false);
56-
addPendingImport(context, sourceFile, "vitest", "beforeAll", false);
57-
addPendingImport(context, sourceFile, "vitest", "describe", false);
58-
addPendingImport(context, sourceFile, "vitest", "test", false);
59-
6051
// go over each expression statement in file
6152
for (const statement of sourceFile.getStatements()) {
6253
if (statement.isKind(SyntaxKind.ExpressionStatement)) {

packages/compas-convert/src/passes/init-ts-morph.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const CONVERT_UTIL_PATH = "./src/compas-convert.ts";
77
export const CONVERT_UTIL = {
88
any: "$ConvertAny",
99
assertNotNil: "assertNotNil",
10+
assertIsAppError: "assertIsAppError",
1011
} as const;
1112

1213
/**
@@ -34,6 +35,8 @@ export async function initTsMorph(context: Context) {
3435
`
3536
// File added by compas-convert
3637
38+
import { AppError } from "@compas/stdlib";
39+
3740
/**
3841
* While migrating, not every type can be resolved or inferred. In these cases, an explicit
3942
* $ConvertAny is added. This solves 2 problems:
@@ -57,6 +60,20 @@ export function assertNotNil<T>(
5760
throw new Error(\`Invariant failed: $\{ message }\`);
5861
}
5962
}
63+
64+
/**
65+
* Asserts that the provided value is of type AppError
66+
*
67+
*/
68+
export function assertIsAppError(
69+
value: unknown,
70+
message?: string,
71+
): asserts value is AppError {
72+
if (!AppError.instanceOf(value)) {
73+
throw new Error(\`Invariant failed: $\{message ?? "Not of type AppError"}\`);
74+
}
75+
}
76+
6077
`,
6178
)
6279
.save();

packages/compas-convert/src/passes/not-nil-checks-in-test-files.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function notNilChecksInTestFiles(context: Context, sourceFile: SourceFile
3939

4040
// The diagnostic text is not always helpful, so retrieve the expression from the file contents.
4141
// - "'user.passwordLogin' is possible 'undefined'"
42-
// - "Object is possible 'undefined''
42+
// - "Object is possible 'undefined'"
4343
const expressionMatch = sourceFile
4444
.getFullText()
4545
.slice(

0 commit comments

Comments
 (0)