fix(error): cache errors instead of evicting on error (#125)

.changeset/fair-ads-kick.md

@@ -0,0 +1,5 @@
+---
+'tsconfck': patch
+---
+
+fix(errors): cache errors instead of evicting cache value on error

docs/api.md

@@ -279,15 +279,17 @@ export class TSConfckCache<T> {
 	hasTSConfigPath(dir: string): boolean;
 	/**
 	 * get cached closest tsconfig for files in dir
-	 * */
+	 * @throws if cached value is an error
+	 */
 	getTSConfigPath(dir: string): Promise<string | null> | string | null;
 	/**
 	 * has parsed tsconfig for file
 	 * */
 	hasParseResult(file: string): boolean;
 	/**
 	 * get parsed tsconfig for file
-	 * */
+	 * @throws if cached value is an error
+	 */
 	getParseResult(file: string): Promise<T> | T;
 }
 ```

packages/tsconfck/src/cache.js

@@ -21,9 +21,15 @@ export class TSConfckCache {
 	 * get cached closest tsconfig for files in dir
 	 * @param {string} dir
 	 * @returns {Promise<string|null>|string|null}
+	 * @throws {unknown} if cached value is an error
 	 */
 	getTSConfigPath(dir) {
-		return this.#tsconfigPaths.get(dir);
+		const value = this.#tsconfigPaths.get(dir);
+		if (value === null || value.length || value.then) {
+			return value;
+		} else {
+			throw value;
+		}
 	}
 
 	/**
@@ -39,9 +45,15 @@ export class TSConfckCache {
 	 * get parsed tsconfig for file
 	 * @param {string} file
 	 * @returns {Promise<T>|T}
+	 * @throws {unknown} if cached value is an error
 	 */
 	getParseResult(file) {
-		return this.#parsed.get(file);
+		const value = this.#parsed.get(file);
+		if (value.then || value.tsconfig) {
+			return value;
+		} else {
+			throw value; // cached error, rethrow
+		}
 	}
 
 	/**
@@ -58,9 +70,9 @@ export class TSConfckCache {
 					this.#parsed.set(file, parsed);
 				}
 			})
-			.catch(() => {
+			.catch((e) => {
 				if (this.#parsed.get(file) === result) {
-					this.#parsed.delete(file);
+					this.#parsed.set(file, e);
 				}
 			});
 	}
@@ -79,9 +91,9 @@ export class TSConfckCache {
 					this.#tsconfigPaths.set(dir, path);
 				}
 			})
-			.catch(() => {
+			.catch((e) => {
 				if (this.#tsconfigPaths.get(dir) === tsconfigPath) {
-					this.#tsconfigPaths.delete(dir);
+					this.#tsconfigPaths.set(dir, e);
 				}
 			});
 	}

packages/tsconfck/src/find.js

@@ -1,6 +1,6 @@
 import path from 'node:path';
 import fs from 'node:fs';
-import { stripNodeModules } from './util.js';
+import { makePromise, stripNodeModules } from './util.js';
 /**
  * find the closest tsconfig.json file
  *
@@ -17,56 +17,50 @@ export async function find(filename, options) {
 	if (cache?.hasTSConfigPath(dir)) {
 		return cache.getTSConfigPath(dir);
 	}
+	const { /** @type {Promise<string|null>} */ promise, resolve, reject } = makePromise();
 	const root = options?.root ? path.resolve(options.root) : null;
-	/** @type {((result: string|null,err?: ErrnoException)=>void)} */
-	let done;
-	/** @type {Promise<string|null> | string | null}*/
-	const promise = new Promise((resolve, reject) => {
-		done = (result, err) => {
-			if (err) {
-				reject(err);
-			} else {
-				resolve(result);
-			}
-		};
-	});
-	findUp(dir, promise, done, options?.cache, root);
+	findUp(dir, { promise, resolve, reject }, options?.cache, root);
 	return promise;
 }
 
 /**
  *
  * @param {string} dir
- * @param {Promise<string|null>} promise
- * @param {((result: string|null,err?: ErrnoException)=>void)} done
+ * @param {{promise:Promise<string|null>,resolve:(result:string|null)=>void,reject:(err:any)=>void}} madePromise
  * @param {import('./cache.js').TSConfckCache}  [cache]
  * @param {string} [root]
  */
-function findUp(dir, promise, done, cache, root) {
+function findUp(dir, { resolve, reject, promise }, cache, root) {
 	const tsconfig = path.join(dir, 'tsconfig.json');
 	if (cache) {
 		if (cache.hasTSConfigPath(dir)) {
-			const cached = cache.getTSConfigPath(dir);
+			let cached;
+			try {
+				cached = cache.getTSConfigPath(dir);
+			} catch (e) {
+				reject(e);
+				return;
+			}
 			if (cached?.then) {
-				/** @type Promise<string|null> */ cached.then(done).catch((err) => done(null, err));
+				cached.then(resolve).catch(reject);
 			} else {
-				done(/**@type {string|null} */ (cached));
+				resolve(cached);
 			}
 		} else {
 			cache.setTSConfigPath(dir, promise);
 		}
 	}
 	fs.stat(tsconfig, (err, stats) => {
 		if (stats && (stats.isFile() || stats.isFIFO())) {
-			done(tsconfig);
+			resolve(tsconfig);
 		} else if (err?.code !== 'ENOENT') {
-			done(null, err);
+			reject(err);
 		} else {
 			let parent;
 			if (root === dir || (parent = path.dirname(dir)) === dir) {
-				done(null);
+				resolve(null);
 			} else {
-				findUp(parent, promise, done, cache, root);
+				findUp(parent, { promise, resolve, reject }, cache, root);
 			}
 		}
 	});

packages/tsconfck/src/parse-native.js

@@ -1,5 +1,6 @@
 import path from 'node:path';
 import {
+	makePromise,
 	loadTS,
 	native2posix,
 	resolveReferencedTSConfigFiles,
@@ -30,33 +31,37 @@ export async function parseNative(filename, options) {
 	if (cache?.hasParseResult(filename)) {
 		return cache.getParseResult(filename);
 	}
-	/** @type {(result: import('./public.d.ts').TSConfckParseNativeResult)=>void}*/
-	let resolveConfigPromise;
-	/** @type {Promise<import('./public.d.ts').TSConfckParseNativeResult>}*/
-	const configPromise = new Promise((r) => {
-		resolveConfigPromise = r;
-	});
-	cache?.setParseResult(filename, configPromise);
-	const tsconfigFile =
-		(await resolveTSConfigJson(filename, cache)) || (await findNative(filename, options));
-	if (!tsconfigFile) {
-		resolveConfigPromise(notFoundResult);
-		return configPromise;
-	}
-
-	/** @type {import('./public.d.ts').TSConfckParseNativeResult} */
-	let result;
-	if (filename !== tsconfigFile && cache?.hasParseResult(tsconfigFile)) {
-		result = await cache.getParseResult(tsconfigFile);
-	} else {
-		const ts = await loadTS();
-		result = await parseFile(tsconfigFile, ts, options, filename === tsconfigFile);
-		await parseReferences(result, ts, options);
-		cache?.setParseResult(tsconfigFile, Promise.resolve(result));
+	const {
+		resolve,
+		reject,
+		/** @type {Promise<import('./public.d.ts').TSConfckParseNativeResult>}*/
+		promise
+	} = makePromise();
+	cache?.setParseResult(filename, promise);
+	try {
+		const tsconfigFile =
+			(await resolveTSConfigJson(filename, cache)) || (await findNative(filename, options));
+		if (!tsconfigFile) {
+			resolve(notFoundResult);
+			return promise;
+		}
+		/** @type {import('./public.d.ts').TSConfckParseNativeResult} */
+		let result;
+		if (filename !== tsconfigFile && cache?.hasParseResult(tsconfigFile)) {
+			result = await cache.getParseResult(tsconfigFile);
+		} else {
+			const ts = await loadTS();
+			result = await parseFile(tsconfigFile, ts, options, filename === tsconfigFile);
+			await parseReferences(result, ts, options);
+			cache?.setParseResult(tsconfigFile, Promise.resolve(result));
+		}
+		//@ts-ignore
+		resolve(resolveSolutionTSConfig(filename, result));
+		return promise;
+	} catch (e) {
+		reject(e);
+		return promise;
 	}
-	//@ts-ignore
-	resolveConfigPromise(resolveSolutionTSConfig(filename, result));
-	return configPromise;
 }
 
 /**

packages/tsconfck/src/parse.js

@@ -4,6 +4,7 @@ import { createRequire } from 'module';
 import { find } from './find.js';
 import { toJson } from './to-json.js';
 import {
+	makePromise,
 	native2posix,
 	resolve2posix,
 	resolveReferencedTSConfigFiles,
@@ -30,30 +31,33 @@ export async function parse(filename, options) {
 	if (cache?.hasParseResult(filename)) {
 		return cache.getParseResult(filename);
 	}
-	/** @type {(result: import('./public.d.ts').TSConfckParseResult)=>void}*/
-	let resolveConfigPromise;
-	/** @type {Promise<import('./public.d.ts').TSConfckParseResult>}*/
-	const configPromise = new Promise((r) => {
-		resolveConfigPromise = r;
-	});
-	cache?.setParseResult(filename, configPromise);
-
-	let tsconfigFile =
-		(await resolveTSConfigJson(filename, cache)) || (await find(filename, options));
-	if (!tsconfigFile) {
-		resolveConfigPromise(not_found_result);
-		return configPromise;
-	}
-
-	let result;
-	if (filename !== tsconfigFile && cache?.hasParseResult(tsconfigFile)) {
-		result = await cache.getParseResult(tsconfigFile);
-	} else {
-		result = await parseFile(tsconfigFile, cache, filename === tsconfigFile);
-		await Promise.all([parseExtends(result, cache), parseReferences(result, cache)]);
+	const {
+		resolve,
+		reject,
+		/** @type {Promise<import('./public.d.ts').TSConfckParseResult>}*/
+		promise
+	} = makePromise();
+	cache?.setParseResult(filename, promise);
+	try {
+		let tsconfigFile =
+			(await resolveTSConfigJson(filename, cache)) || (await find(filename, options));
+		if (!tsconfigFile) {
+			resolve(not_found_result);
+			return promise;
+		}
+		let result;
+		if (filename !== tsconfigFile && cache?.hasParseResult(tsconfigFile)) {
+			result = await cache.getParseResult(tsconfigFile);
+		} else {
+			result = await parseFile(tsconfigFile, cache, filename === tsconfigFile);
+			await Promise.all([parseExtends(result, cache), parseReferences(result, cache)]);
+		}
+		resolve(resolveSolutionTSConfig(filename, result));
+		return promise;
+	} catch (e) {
+		reject(e);
+		return promise;
 	}
-	resolveConfigPromise(resolveSolutionTSConfig(filename, result));
-	return configPromise;
 }
 
 /**

packages/tsconfck/src/util.js

@@ -13,6 +13,19 @@ const DEFAULT_EXTENSIONS_RE_GROUP = `\\.(?:${DEFAULT_EXTENSIONS.map((ext) => ext
 
 const IS_POSIX = path.posix.sep === path.sep;
 
+/**
+ * @template T
+ * @returns {{resolve:(result:T)=>void, reject:(error:any)=>void, promise: Promise<T>}}
+ */
+export function makePromise() {
+	let resolve, reject;
+	const promise = new Promise((res, rej) => {
+		resolve = res;
+		reject = rej;
+	});
+	return { promise, resolve, reject };
+}
+
 /**
  * loads typescript async to avoid direct dependency
  * @returns {Promise<any>}

packages/tsconfck/tests/parse-native.js

@@ -194,6 +194,35 @@ describe('parse', () => {
 		}
 	});
 
+	it('should cache error for invalid tsconfig.json', async () => {
+		let samples = await globFixtures('parse/invalid/**/tsconfig.json');
+		const excluded = [
+			path.join('extends-fallback-not-found', 'dir'),
+			path.join('invalid', 'tsconfig.json') // directory, not a file, does
+		];
+		samples = samples.filter((sample) => !excluded.some((excluded) => sample.includes(excluded)));
+		const cache = new TSConfckCache();
+		for (const filename of samples) {
+			expect(cache.hasParseResult(filename)).toBe(false);
+			let error;
+			try {
+				await parseNative(filename, { cache });
+				expect.unreachable(`parse for ${filename} did not reject`);
+			} catch (e) {
+				expect(e).toBeInstanceOf(TSConfckParseNativeError);
+				expect(e.tsconfigFile).toBe(filename);
+				error = e;
+			}
+			expect(cache.hasParseResult(filename)).toBe(true);
+			try {
+				await cache.getParseResult(filename);
+				expect.unreachable(`cache.getParseResult for ${filename} did not reject`);
+			} catch (e) {
+				expect(e).toBe(error);
+			}
+		}
+	});
+
 	it('should prevent typescript file scanning when ignoreSourceFiles: true is set', async () => {
 		// use the more interesting samples with extensions and solution-style
 		const samples = [

packages/tsconfck/tests/parse.js

@@ -192,4 +192,33 @@ describe('parse', () => {
 			}
 		}
 	});
+
+	it('should cache error for invalid tsconfig.json', async () => {
+		let samples = await globFixtures('parse/invalid/**/tsconfig.json');
+		const excluded = [
+			path.join('extends-fallback-not-found', 'dir'),
+			path.join('invalid', 'tsconfig.json') // directory, not a file, does
+		];
+		samples = samples.filter((sample) => !excluded.some((excluded) => sample.includes(excluded)));
+		const cache = new TSConfckCache();
+		for (const filename of samples) {
+			expect(cache.hasParseResult(filename)).toBe(false);
+			let error;
+			try {
+				await parse(filename, { cache });
+				expect.unreachable(`parse for ${filename} did not reject`);
+			} catch (e) {
+				expect(e).toBeInstanceOf(TSConfckParseError);
+				expect(e.tsconfigFile).toBe(filename);
+				error = e;
+			}
+			expect(cache.hasParseResult(filename)).toBe(true);
+			try {
+				await cache.getParseResult(filename);
+				expect.unreachable(`cache.getParseResult for ${filename} did not reject`);
+			} catch (e) {
+				expect(e).toBe(error);
+			}
+		}
+	});
 });