Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

processors/typescript: depend on files listed in includes in nearest tsconfig.json #17

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions __tests__/fixtures/a/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/built-ins/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/directive/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/entry/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/feature-storybook-ref/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/framework/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/modules/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
3 changes: 3 additions & 0 deletions __tests__/fixtures/snapshots/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
Empty file.
3 changes: 3 additions & 0 deletions __tests__/fixtures/tsconfigs/decls/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
Empty file.
Empty file.
4 changes: 4 additions & 0 deletions __tests__/fixtures/tsconfigs/extends/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"extends": "../project/tsconfig.json",
"include": ["./**/*.*"]
}
Empty file.
3 changes: 3 additions & 0 deletions __tests__/fixtures/tsconfigs/glob/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["../decls/*.d.ts", "./**/*.*"]
}
Empty file.
Empty file.
3 changes: 3 additions & 0 deletions __tests__/fixtures/tsconfigs/mixed/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["./**/*.*"]
}
Empty file.
Empty file.
3 changes: 3 additions & 0 deletions __tests__/fixtures/tsconfigs/project/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["../decls/one.d.ts", "./**/*.*"]
}
35 changes: 35 additions & 0 deletions __tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,4 +499,39 @@ describe('dependencyTree', () => {
).rejects.toThrowErrorMatchingSnapshot();
});
});

describe('tsconfig', () => {
beforeEach(() => {
dependencyTree = new DependencyTree([fixture('tsconfigs')]);
});

it('adds files in `include` from nearest tsconfig.json', async () => {
expect.hasAssertions();
const result = await dependencyTree.gather();
const decl1 = fixture('tsconfigs', 'decls', 'one.d.ts');
const decl2 = fixture('tsconfigs', 'decls', 'two.d.ts');
expect(result).toStrictEqual({
missing: new Map(),
resolved: new Map([
[fixture('tsconfigs', 'project', 'project.ts'), new Set([decl1])],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[
fixture('tsconfigs', 'project', 'nested', 'nested.ts'),
new Set([decl1]),
],
[fixture('tsconfigs', 'glob', 'glob.ts'), new Set([decl1, decl2])],
// "includes" is overwritten in the child config
[fixture('tsconfigs', 'extends', 'extends.ts'), new Set([])],
// decls in the same dir that includes './**.*.*' depend on one another
[fixture('tsconfigs', 'decls', 'one.d.ts'), new Set([decl2])],
[fixture('tsconfigs', 'decls', 'two.d.ts'), new Set([decl1])],
Comment on lines +525 to +526
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seemed a little odd to me at first but it makes sense. If I have decls/a.d.ts and decls/b.d.ts which uses types from a, then any file that uses types from b should transitively depend on a.

// files can depend on decls in the same dir
[
fixture('tsconfigs', 'mixed', 'source.ts'),
new Set([fixture('tsconfigs', 'mixed', 'decl.d.ts')]),
],
[fixture('tsconfigs', 'mixed', 'decl.d.ts'), new Set([])],
]),
});
});
});
});
55 changes: 55 additions & 0 deletions src/processors/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ export class TypeScriptFileProcessor implements FileProcessor {
dependencyTree: DependencyTree,
): Promise<Set<Path>> {
const importedFiles = new Set<Path>();

// TypeScript files can *implicitly* depend on .d.ts files. We discover
// these files by extracting them from the nearest tsconfig.json file.
// These do not need to be processed further since they have already been fully
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are sure these are absolute paths at all times?
What happens if a non-existent file is referenced, does it end up in the unresolvable set of files correctly? Did you add a test or if not, could you please?

// resolved by the typescript compiler.
const includes = await this.includesFromNearestTsconfigFile(file);
for (const include of includes) {
importedFiles.add(include);
}

const filesList = ts
.preProcessFile(contents, true, true)
.importedFiles.map(({ fileName }) => fileName);
Expand Down Expand Up @@ -189,6 +199,51 @@ export class TypeScriptFileProcessor implements FileProcessor {
},
);

private async includesFromNearestTsconfigFile(file: Path): Promise<string[]> {
const tsconfigPath = ts.findConfigFile(
path.dirname(file),
ts.sys.fileExists,
);
if (!tsconfigPath) {
throw new Error(
`could not find a tsconfig file for '${path.relative(
this.rootDir,
file,
)}'`,
);
}
if (!tsconfigPath.startsWith(this.rootDir)) {
throw new Error(
`invalid tsconfig file '${path.relative(
this.rootDir,
tsconfigPath,
)}' found for '${path.relative(
this.rootDir,
file,
)}'. expected a tsconfig file inside the project root '${
this.rootDir
}'`,
);
}

const json = ts.parseJsonText(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much nicer :)

tsconfigPath,
await fs.promises.readFile(tsconfigPath, 'utf-8'),
);
const parsed = ts.parseJsonSourceFileConfigFileContent(
json,
ts.sys,
path.dirname(tsconfigPath),
);
return parsed.fileNames.filter(
(fileName) =>
// only include .d.ts files. all other references should be made using `import` or `require`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think standard ts(x) files can be referenced with types in them? Can you check? Maybe it makes sense for us to not filter this at all and just take the references verbatim?

fileName.endsWith('.d.ts') &&
// files do not depend on themselves
fileName !== file,
);
}

// Finds an implicit 'import' in the entry point object literal, like:
//
// export const entryPoint: DynamicEntryPoint = {
Expand Down