From b3d9113dd8c522b5542b7b28f23a8835467faf21 Mon Sep 17 00:00:00 2001 From: dominikg Date: Mon, 4 Sep 2023 21:09:03 +0200 Subject: [PATCH] fix(error): cache errors instead of evicting on error --- .changeset/fair-ads-kick.md | 5 +++ docs/api.md | 6 ++- packages/tsconfck/src/cache.js | 24 ++++++++--- packages/tsconfck/src/find.js | 42 ++++++++---------- packages/tsconfck/src/parse-native.js | 57 ++++++++++++++----------- packages/tsconfck/src/parse.js | 50 ++++++++++++---------- packages/tsconfck/src/util.js | 13 ++++++ packages/tsconfck/tests/parse-native.js | 29 +++++++++++++ packages/tsconfck/tests/parse.js | 29 +++++++++++++ packages/tsconfck/types/index.d.ts | 6 ++- packages/tsconfck/types/index.d.ts.map | 2 +- 11 files changed, 179 insertions(+), 84 deletions(-) create mode 100644 .changeset/fair-ads-kick.md diff --git a/.changeset/fair-ads-kick.md b/.changeset/fair-ads-kick.md new file mode 100644 index 0000000..c068914 --- /dev/null +++ b/.changeset/fair-ads-kick.md @@ -0,0 +1,5 @@ +--- +'tsconfck': patch +--- + +fix(errors): cache errors instead of evicting cache value on error diff --git a/docs/api.md b/docs/api.md index f47d604..b0aba82 100644 --- a/docs/api.md +++ b/docs/api.md @@ -279,7 +279,8 @@ export class TSConfckCache { 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; /** * has parsed tsconfig for file @@ -287,7 +288,8 @@ export class TSConfckCache { hasParseResult(file: string): boolean; /** * get parsed tsconfig for file - * */ + * @throws if cached value is an error + */ getParseResult(file: string): Promise | T; } ``` diff --git a/packages/tsconfck/src/cache.js b/packages/tsconfck/src/cache.js index a964013..e556f35 100644 --- a/packages/tsconfck/src/cache.js +++ b/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} + * @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} + * @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); } }); } diff --git a/packages/tsconfck/src/find.js b/packages/tsconfck/src/find.js index d7e2506..aec54c8 100644 --- a/packages/tsconfck/src/find.js +++ b/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,40 +17,34 @@ export async function find(filename, options) { if (cache?.hasTSConfigPath(dir)) { return cache.getTSConfigPath(dir); } + const { /** @type {Promise} */ 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}*/ - 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} promise - * @param {((result: string|null,err?: ErrnoException)=>void)} done + * @param {{promise:Promise,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 */ 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); @@ -58,15 +52,15 @@ function findUp(dir, promise, done, cache, root) { } 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); } } }); diff --git a/packages/tsconfck/src/parse-native.js b/packages/tsconfck/src/parse-native.js index 9b48bc7..e2f68d5 100644 --- a/packages/tsconfck/src/parse-native.js +++ b/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}*/ - 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}*/ + 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; } /** diff --git a/packages/tsconfck/src/parse.js b/packages/tsconfck/src/parse.js index f9d6431..0127da9 100644 --- a/packages/tsconfck/src/parse.js +++ b/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}*/ - 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}*/ + 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; } /** diff --git a/packages/tsconfck/src/util.js b/packages/tsconfck/src/util.js index 01af844..e4381c7 100644 --- a/packages/tsconfck/src/util.js +++ b/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}} + */ +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} diff --git a/packages/tsconfck/tests/parse-native.js b/packages/tsconfck/tests/parse-native.js index 4ae86ee..798eeaf 100644 --- a/packages/tsconfck/tests/parse-native.js +++ b/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 = [ diff --git a/packages/tsconfck/tests/parse.js b/packages/tsconfck/tests/parse.js index 3096f6e..284a7f4 100644 --- a/packages/tsconfck/tests/parse.js +++ b/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); + } + } + }); }); diff --git a/packages/tsconfck/types/index.d.ts b/packages/tsconfck/types/index.d.ts index e30afe6..4f0e704 100644 --- a/packages/tsconfck/types/index.d.ts +++ b/packages/tsconfck/types/index.d.ts @@ -43,7 +43,8 @@ declare module 'tsconfck' { 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; /** * has parsed tsconfig for file @@ -51,7 +52,8 @@ declare module 'tsconfck' { hasParseResult(file: string): boolean; /** * get parsed tsconfig for file - * */ + * @throws if cached value is an error + */ getParseResult(file: string): Promise | T; private setParseResult; diff --git a/packages/tsconfck/types/index.d.ts.map b/packages/tsconfck/types/index.d.ts.map index 0b5b73a..4eb4483 100644 --- a/packages/tsconfck/types/index.d.ts.map +++ b/packages/tsconfck/types/index.d.ts.map @@ -38,5 +38,5 @@ null, null ], - "mappings": ";;;;;;;;iBAUsBA,IAAIA;;;;;;;;iBCWJC,OAAOA;;;;;;;iBCRbC,MAAMA;;;;;;;;;;iBCDAC,UAAUA;cCXnBC,aAAaA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;iBCyBJC,KAAKA;cAqTdC,kBAAkBA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;iBCrTTC,WAAWA;cAyNpBC,wBAAwBA;;;;;;;;;;;;;;;;;;;;;;;;;WCjPpBC,mBAAmBA;;;;;;;;;;;;;;;;;;;;;;;WAuBnBC,oBAAoBA;;;;WAIpBC,sBAAsBA;;;;;;;;;WAStBC,mBAAmBA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;WA6BnBC,0BAA0BA;;;;;;;;;;;;WAY1BC,yBAAyBA" + "mappings": ";;;;;;;;iBAUsBA,IAAIA;;;;;;;;iBCWJC,OAAOA;;;;;;;iBCRbC,MAAMA;;;;;;;;;;iBCDAC,UAAUA;cCXnBC,aAAaA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;iBC0BJC,KAAKA;cAwTdC,kBAAkBA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;iBCxTTC,WAAWA;cA6NpBC,wBAAwBA;;;;;;;;;;;;;;;;;;;;;;;;;WCtPpBC,mBAAmBA;;;;;;;;;;;;;;;;;;;;;;;WAuBnBC,oBAAoBA;;;;WAIpBC,sBAAsBA;;;;;;;;;WAStBC,mBAAmBA;;;;;;;;;;;;;;;;;;;;;;;;;;;;;WA6BnBC,0BAA0BA;;;;;;;;;;;;WAY1BC,yBAAyBA" } \ No newline at end of file