Skip to content

Commit

Permalink
fix: add internal property '_isRootFile_' for each cache to prevent i…
Browse files Browse the repository at this point in the history
…nto deadlock with cache
  • Loading branch information
daihere1993 committed Mar 3, 2024
1 parent 404d909 commit 0d1c791
Show file tree
Hide file tree
Showing 26 changed files with 287 additions and 10 deletions.
5 changes: 4 additions & 1 deletion packages/tsconfck/src/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ export class TSConfckCache {
* @internal
* @private
* @param file
* @param {boolean} isRootFile is it the file which involking the parse() api, used to distinguish the normal cache which only parsed by parseFile()
* @param {Promise<T>} result
*/
setParseResult(file, result) {
setParseResult(file, result, isRootFile = false) {
// _isRootFile_ is a internal property, used to prevent deadlock with cache
result._isRootFile_ = isRootFile;
this.#parsed.set(file, result);
result
.then((parsed) => {
Expand Down
15 changes: 11 additions & 4 deletions packages/tsconfck/src/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export async function parse(filename, options) {
/** @type {Promise<import('./public.d.ts').TSConfckParseResult>}*/
promise
} = makePromise();
cache?.setParseResult(filename, promise);
cache?.setParseResult(filename, promise, true);
try {
let tsconfigFile =
(await resolveTSConfigJson(filename, cache)) || (await find(filename, options));
Expand Down Expand Up @@ -76,7 +76,7 @@ async function getParsedDeep(filename, cache, options) {
parseExtends(result, cache),
parseReferences(result, options)
]).then(() => result);
cache.setParseResult(filename, promise);
cache.setParseResult(filename, promise, true);
return promise;
}
return result;
Expand All @@ -90,7 +90,11 @@ async function getParsedDeep(filename, cache, options) {
* @returns {Promise<import('./public.d.ts').TSConfckParseResult>}
*/
async function parseFile(tsconfigFile, cache, skipCache) {
if (!skipCache && cache?.hasParseResult(tsconfigFile)) {
if (
!skipCache &&
cache?.hasParseResult(tsconfigFile) &&
!cache.getParseResult(tsconfigFile)._isRootFile_
) {
return cache.getParseResult(tsconfigFile);
}
const promise = fs
Expand All @@ -112,7 +116,10 @@ async function parseFile(tsconfigFile, cache, skipCache) {
e
);
});
if (!skipCache) {
if (
!skipCache &&
(!cache?.hasParseResult(tsconfigFile) || !cache.getParseResult(tsconfigFile)._isRootFile_)
) {
cache?.setParseResult(tsconfigFile, promise);
}
return promise;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function foo() {
return 'foo';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"include": ["./foo.ts"],
"extends": "../tsconfig",
"compilerOptions": {
"composite": true,
"strict": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { foo } from '../src/foo';
import * as assert from 'assert';

function test() {
const actual = foo();
const expected = 'foo';
assert.strictEqual(actual, expected);
}
test();
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"include": ["./*.test.ts"],
"extends": "../tsconfig",
"compilerOptions": {
"composite": true,
"strict": false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"references": [
{"path": "./src/tsconfig.src.json"},
{"path": "./tests/tsconfig.test.json"}
]
}
49 changes: 44 additions & 5 deletions packages/tsconfck/tests/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,23 @@ describe('parse', () => {
// use the more interesting samples with extensions and solution-style
const samples = [
...(await globFixtures('parse/valid/with_extends/**/tsconfig.json')),
...(await globFixtures('parse/solution/**/*.ts'))
...(await globFixtures('parse/solution/**/*.ts')),
...(await globFixtures('parse/solution/**/*.json'))
];
const cache = new TSConfckCache();
for (const filename of samples) {
// these 3 directories have extends declarations that lead to the configs being parsed from extends first and cached before explicit access
const expectedHasParseResult = ['dotdot', 'nested', 'nested/src'].some((dir) =>
filename.endsWith(posix2native(`with_extends/${dir}/tsconfig.json`))
);
// these configs are expected to exist in cache already
// because they have been parsed by being extended in previous samples
const extendedSamples = [
'with_extends/dotdot/tsconfig.json',
'with_extends/nested/tsconfig.json',
'with_extends/nested/src/tsconfig.json',
'solution/jsconfig/jsconfig.src.json',
'solution/mixed/tsconfig.src.json',
'solution/simple/tsconfig.base.json',
'solution/referenced-extends-original/tsconfig.json'
].map(posix2native);
const expectedHasParseResult = extendedSamples.some((sample) => filename.endsWith(sample));
expect(
cache.hasParseResult(filename),
`cache does ${!expectedHasParseResult ? 'not ' : ' '}exist for ${filename}`
Expand Down Expand Up @@ -189,6 +198,36 @@ describe('parse', () => {
}
});

it('should not lock up parsing in parallel with cache', async () => {
const samples = [
...(await globFixtures('parse/valid/**/*.ts')),
...(await globFixtures('parse/valid/**/*.json')),
...(await globFixtures('parse/solution/**/*.ts')),
...(await globFixtures('parse/solution/**/*.json'))
];
expect(samples.length).toBeGreaterThan(10);
const cache = new TSConfckCache();
const unfinished = new Set(samples);
try {
const results = await Promise.race([
Promise.all(
samples.map((s) => {
const sample = s;
return parse(sample, { cache }).then(() => unfinished.delete(sample));
})
),
new Promise((_, reject) => setTimeout(reject, 500))
]);
expect(results.length).toBe(samples.length);
} catch (e) {
expect.fail(
`did not process all files, some of these are blocking each other:\n${[...unfinished].join(
'\n'
)}\n`
);
}
});

it('should resolve with tsconfig that is isomorphic', async () => {
const tempDir = await copyFixtures(
'parse/valid',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"compilerOptions": {
"allowJs": true,
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"files": [],
"include": [],
"references": [
{
"path": "./jsconfig.src.json"
},
{
"path": "./jsconfig.test.json"
}
],
"compilerOptions": {
"allowJs": true,
"maxNodeModuleJsDepth": 2,
"allowSyntheticDefaultImports": true,
"skipLibCheck": true,
"noEmit": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"extends": "./jsconfig.base",
"include": [
"src/**/*"
],
"exclude": [
"src/**/*.spec.js"
],
"compilerOptions": {
"strict": true,
"allowJs": true,
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"extends": "./jsconfig.base",
"include": [
"src/**/*.spec.js"
],
"compilerOptions": {
"strict": false,
"allowJs": true,
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"compilerOptions": {
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"extends": "./tsconfig.base",
"include": [
"src/**/*"
],
"exclude": [
"src/**/*.spec.ts"
],
"compilerOptions": {
"strict": true,
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "./tsconfig.base",
"include": [
"src/**/*.spec.ts"
],
"compilerOptions": {
"strict": false,
"composite": true,
"strictNullChecks": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
],
"files": [],
"include": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"include": [
"./foo.ts"
],
"extends": "../tsconfig",
"compilerOptions": {
"composite": true,
"strict": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
],
"files": [],
"include": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"include": [
"./*.test.ts"
],
"extends": "../tsconfig",
"compilerOptions": {
"composite": true,
"strict": false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"references": [
{
"path": "./src/tsconfig.src.json"
},
{
"path": "./tests/tsconfig.test.json"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"compilerOptions": {
"composite": true,
"strictNullChecks": true
}
}

0 comments on commit 0d1c791

Please sign in to comment.