Skip to content

Commit

Permalink
fix: switch find cache to lazy promise to ensure stat is called at ma…
Browse files Browse the repository at this point in the history
…x once per dir
  • Loading branch information
dominikg committed Aug 28, 2023
1 parent caffc45 commit 99cd4f2
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 46 deletions.
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ export class TSConfckCache {
/**
* get cached closest tsconfig for files in dir
* */
getTSConfigPath(dir: string): string;
getTSConfigPath(dir: string): Promise<string>;
/**
* has parsed tsconfig for file
* */
Expand Down
16 changes: 7 additions & 9 deletions packages/tsconfck/src/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ export class TSConfckCache {
/**
* get cached closest tsconfig for files in dir
* @param {string} dir
* @returns {string}
* @returns {Promise<string>}
*/
getTSConfigPath(dir) {
async getTSConfigPath(dir) {
return this.#tsconfigPaths.get(dir);
}

Expand Down Expand Up @@ -56,20 +56,18 @@ export class TSConfckCache {
/**
* @internal
* @private
* @param {string} tsconfigPath
* @param {string[]} directories
* @param {string} dir
* @param {Promise<string>} tsconfigPath
*/
setTSConfigPath(tsconfigPath, directories) {
for (const dir of directories) {
this.#tsconfigPaths.set(dir, tsconfigPath);
}
setTSConfigPath(dir, tsconfigPath) {
this.#tsconfigPaths.set(dir, tsconfigPath);
}

/**
* map directories to their closest tsconfig.json
* @internal
* @private
* @type{Map<string,string>}
* @type{Map<string,Promise<string>>}
*/
#tsconfigPaths = new Map();

Expand Down
7 changes: 5 additions & 2 deletions packages/tsconfck/src/find-native.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export async function findNative(filename, options) {
const cache = options?.cache;
const root = options?.root ? path.resolve(options.root) : undefined;
if (cache?.hasTSConfigPath(fileDir)) {
const tsconfigFile = cache.getTSConfigPath(fileDir);
const tsconfigFile = await cache.getTSConfigPath(fileDir);
if (!tsconfigFile) {
throw new Error(`no tsconfig file found for ${filename}`);
}
Expand Down Expand Up @@ -66,5 +66,8 @@ function cache_result(tsconfigFile, fileDir, cache, root) {
dir = parent;
}
}
cache.setTSConfigPath(tsconfigFile, directories);
const p = Promise.resolve(tsconfigFile);
directories.forEach((d) => {
cache.setTSConfigPath(d, p);
});
}
38 changes: 21 additions & 17 deletions packages/tsconfck/src/find.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,42 @@ import { promises as fs } from 'fs';
export async function find(filename, options) {
const cache = options?.cache;
let dir = path.dirname(path.resolve(filename));
if (cache?.hasTSConfigPath(dir)) {
return cache.getTSConfigPath(dir);
}
const root = options?.root ? path.resolve(options.root) : null;
/** @type {string[]} */
const visited = [];
let found;

/** @type {(result: string|null)=>void}*/
let resolvePathPromise;
/** @type {Promise<string|null> | string | null}*/
const pathPromise = new Promise((r) => {
resolvePathPromise = r;
});
while (dir) {
if (cache?.hasTSConfigPath(dir)) {
found = cache.getTSConfigPath(dir);
break;
if (cache) {
if (cache.hasTSConfigPath(dir)) {
cache.getTSConfigPath(dir).then(resolvePathPromise);
return pathPromise;
} else {
cache.setTSConfigPath(dir, pathPromise);
}
}
visited.push(dir);
const tsconfig = await tsconfigInDir(dir);
if (tsconfig) {
found = tsconfig;
break;
resolvePathPromise(tsconfig);
return pathPromise;
} else {
const parent = path.dirname(dir);
if (root === dir || parent === dir) {
// reached root
found = null;
break;
} else {
dir = parent;
}
}
}
if (cache && visited.length) {
cache.setTSConfigPath(found, visited);
}
if (!found) {
throw new Error(`no tsconfig file found for ${filename}`);
}
return found;
resolvePathPromise(null);
throw new Error(`no tsconfig file found for ${filename}`);
}

/**
Expand Down
11 changes: 1 addition & 10 deletions packages/tsconfck/tests/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,12 @@ describe('cache', () => {
beforeEach(() => {
cache = new TSConfckCache();
});
describe('setTSConfigPath', () => {
it('should add entries for every directory', () => {
expect(cache.hasTSConfigPath('bar')).toBe(false);
expect(cache.hasTSConfigPath('baz')).toBe(false);
cache.setTSConfigPath('foo', ['bar', 'baz']);
expect(cache.hasTSConfigPath('bar')).toBe(true);
expect(cache.hasTSConfigPath('baz')).toBe(true);
});
});
describe('clear', () => {
it('should remove all data', () => {
const result = /**@type TSConfckParseResult */ ({});
expect(cache.hasParseResult('file')).toBe(false);
cache.setParseResult('file', result);
cache.setTSConfigPath('foo', ['bar']);
cache.setTSConfigPath('bar', Promise.resolve('bar'));
expect(cache.hasParseResult('file')).toBe(true);
expect(cache.hasTSConfigPath('bar')).toBe(true);
cache.clear();
Expand Down
10 changes: 6 additions & 4 deletions packages/tsconfck/tests/find-native.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,16 @@ describe('find-native', () => {
}
const dir = path.dirname(absoluteTS);
expect(cache.hasTSConfigPath(dir)).toBe(true);
expect(cache.getTSConfigPath(dir)).toBe(expected);
expect(await cache.getTSConfigPath(dir)).toBe(expected);
const parent = path.dirname(dir);
expect(cache.hasTSConfigPath(parent)).toBe(true);
expect(cache.getTSConfigPath(parent)).toBe(expected);
expect(await cache.getTSConfigPath(parent)).toBe(expected);
const root = path.dirname(real);
expect(cache.hasTSConfigPath(root)).toBe(true);
expect(cache.getTSConfigPath(root)).toBe(expected);
cache.setTSConfigPath('fake', [dir, parent, root]);
expect(await cache.getTSConfigPath(root)).toBe(expected);
[dir, parent, root].forEach((d) => {
cache.setTSConfigPath(d, Promise.resolve('fake'));
});
for (const input of inputs) {
expect(await findNative(input, { cache }), `input: ${input}`).toBe('fake');
}
Expand Down
4 changes: 2 additions & 2 deletions packages/tsconfck/tests/find.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ describe('find', () => {
const fake = absFixture(`${fixtureDir}/a/tsconfig.json`);

const cache = new TSConfckCache();
cache.setTSConfigPath(fake, [path.dirname(fake)]);
cache.setTSConfigPath(path.dirname(fake), Promise.resolve(fake));

for (const input of inputs) {
expect(await find(input), `input: ${input}`).toBe(real);
expect(await find(input, { cache }), `input: ${input}`).toBe(fake);
}
const added_key = path.dirname(absoluteTS);
expect(cache.hasTSConfigPath(added_key)).toBe(true);
expect(cache.getTSConfigPath(added_key)).toBe(fake);
expect(await cache.getTSConfigPath(added_key)).toBe(fake);
});

it('should reject when no tsconfig file was found', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/tsconfck/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ declare module 'tsconfck' {
/**
* get cached closest tsconfig for files in dir
* */
getTSConfigPath(dir: string): string;
getTSConfigPath(dir: string): Promise<string>;
/**
* has parsed tsconfig for file
* */
Expand Down

0 comments on commit 99cd4f2

Please sign in to comment.