Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fair-ads-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'tsconfck': patch
---

fix(errors): cache errors instead of evicting cache value on error
6 changes: 4 additions & 2 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
```
24 changes: 18 additions & 6 deletions packages/tsconfck/src/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

/**
Expand All @@ -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
}
}

/**
Expand All @@ -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);
}
});
}
Expand All @@ -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);
}
});
}
Expand Down
42 changes: 18 additions & 24 deletions packages/tsconfck/src/find.js
Original file line number Diff line number Diff line change
@@ -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
*
Expand All @@ -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);
}
}
});
Expand Down
57 changes: 31 additions & 26 deletions packages/tsconfck/src/parse-native.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from 'node:path';
import {
makePromise,
loadTS,
native2posix,
resolveReferencedTSConfigFiles,
Expand Down Expand Up @@ -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;
}

/**
Expand Down
50 changes: 27 additions & 23 deletions packages/tsconfck/src/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createRequire } from 'module';
import { find } from './find.js';
import { toJson } from './to-json.js';
import {
makePromise,
native2posix,
resolve2posix,
resolveReferencedTSConfigFiles,
Expand All @@ -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;
}

/**
Expand Down
13 changes: 13 additions & 0 deletions packages/tsconfck/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>}
Expand Down
29 changes: 29 additions & 0 deletions packages/tsconfck/tests/parse-native.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
29 changes: 29 additions & 0 deletions packages/tsconfck/tests/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
});
});
Loading