Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Add typeroots support #343

Merged
merged 12 commits into from
Sep 27, 2017
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
63 changes: 62 additions & 1 deletion src/project-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ export class ProjectManager implements Disposable {
*/
private ensuredModuleStructure?: Observable<never>;

/**
* Observable that completes when extra dependencies pointed to by tsconfig.json have been loaded.
*/
private ensuredConfigDependencies?: Observable<never>;

/**
* Observable that completes when `ensureAllFiles` completed
*/
Expand Down Expand Up @@ -252,6 +257,7 @@ export class ProjectManager implements Disposable {
*/
invalidateModuleStructure(): void {
this.ensuredModuleStructure = undefined;
this.ensuredConfigDependencies = undefined;
this.ensuredAllFiles = undefined;
this.ensuredOwnFiles = undefined;
}
Expand Down Expand Up @@ -321,6 +327,7 @@ export class ProjectManager implements Disposable {
span.addTags({ uri, maxDepth });
ignore.add(uri);
return this.ensureModuleStructure(span)
.concat(Observable.defer(() => this.ensureConfigDependencies()))
Copy link
Contributor

@felixfbecker felixfbecker Sep 20, 2017

Choose a reason for hiding this comment

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

Does this have a performance impact? I think it probably has, as you are iterating in ensureConfigDependencies() over every single file in the workspace, and ensureReferencedFiles() is called recursively for every referenced file, so you are repeating that iteration every time. I think ensureConfigDependencies() needs to memoize its return value in the same way ensureAllFiles() does, or you could just add that logic to ensureModuleStructure() (which would already is memoized, has memoize cache invalidation logic and already iterates over all files so you'd only have to iterate once). Optionally, you could try this then on a very big TS project and see if it degrades perf in any way

// If max depth was reached, don't go any further
.concat(Observable.defer(() => maxDepth === 0 ? Observable.empty<never>() : this.resolveReferencedFiles(uri)))
// Prevent cycles
Expand All @@ -337,6 +344,39 @@ export class ProjectManager implements Disposable {
});
}

/**
* Determines if a tsconfig/jsconfig needs additional declaration files loaded.
* @param filePath
*/
isConfigDependency(filePath: string): boolean {
for (const config of this.configurations()) {
config.ensureConfigFile();
if (config.isExpectedDeclarationFile(filePath)) {
return true;
}
}
return false;
}

/**
* Loads files determined by tsconfig to be needed into the file system
*/
ensureConfigDependencies(childOf = new Span()): Observable<never> {
return traceObservable('Ensure config dependencies', childOf, span => {
if (!this.ensuredConfigDependencies) {
this.ensuredConfigDependencies = observableFromIterable(this.inMemoryFs.uris())
.filter(uri => this.isConfigDependency(uri2path(uri)))
.mergeMap(uri => this.updater.ensure(uri))
.do(noop, err => {
this.ensuredConfigDependencies = undefined;
})
.publishReplay()
.refCount() as Observable<never>;
}
return this.ensuredConfigDependencies;
});
}

/**
* Invalidates a cache entry for `resolveReferencedFiles` (e.g. because the file changed)
*
Expand Down Expand Up @@ -734,6 +774,11 @@ export class ProjectConfiguration {
*/
private expectedFilePaths = new Set<string>();

/**
* List of resolved extra root directories to allow global type declaration files to be loaded from.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this restricted to global typings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From https://www.typescriptlang.org/docs/handbook/tsconfig-json.html:

By default all visible “@types” packages are included in your compilation.
If typesRoots is specified, only packages under typeRoots will be included.
Keep in mind that automatic inclusion is only important if you’re using files with global declarations (as opposed to files declared as modules)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They clarify in that page as well:

If you use an import "foo" statement, for instance, TypeScript may still look through node_modules & node_modules/@types folders to find the foo package.

*/
private typeRoots: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

is the order important? if not, use a Set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this.typeRoots.some(root => fileName.startsWith(root)); is a good fit for Set?


/**
* @param fs file system to use
* @param documentRegistry Shared DocumentRegistry that manages SourceFile objects
Expand Down Expand Up @@ -838,6 +883,11 @@ export class ProjectConfiguration {
this.expectedFilePaths = new Set(configParseResult.fileNames);

const options = configParseResult.options;
const pathResolver = /^[a-z]:\//i.test(base) ? path.win32 : path.posix;
this.typeRoots = options.typeRoots ?
options.typeRoots.map((r: string) => pathResolver.resolve(this.rootFilePath, r)) :
[];

if (/(^|\/)jsconfig\.json$/.test(this.configFilePath)) {
options.allowJs = true;
}
Expand All @@ -864,6 +914,16 @@ export class ProjectConfiguration {

private ensuredBasicFiles = false;

/**
* Determines if a fileName is a declaration file within expected files or type roots
* @param fileName
*/
public isExpectedDeclarationFile(fileName: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

return type

return isDeclarationFile(fileName) &&
(this.expectedFilePaths.has(toUnixPath(fileName)) ||
this.typeRoots.some(root => fileName.startsWith(root)));
}

/**
* Ensures we added basic files (global TS files, dependencies, declarations)
*/
Expand All @@ -882,7 +942,8 @@ export class ProjectConfiguration {
// Add all global declaration files from the workspace and all declarations from the project
for (const uri of this.fs.uris()) {
const fileName = uri2path(uri);
if (isGlobalTSFile(fileName) || (isDeclarationFile(fileName) && this.expectedFilePaths.has(toUnixPath(fileName)))) {
if (isGlobalTSFile(fileName) ||
this.isExpectedDeclarationFile(fileName)) {
const sourceFile = program.getSourceFile(fileName);
if (!sourceFile) {
this.getHost().addFile(fileName);
Expand Down
20 changes: 20 additions & 0 deletions src/test/project-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,26 @@ describe('ProjectManager', () => {
assert.isDefined(configs.find(config => config.configFilePath === '/foo/tsconfig.json'));
});

describe('ensureBasicFiles', () => {
beforeEach(async () => {
memfs = new InMemoryFileSystem('/');
const localfs = new MapFileSystem(new Map([
['file:///project/package.json', '{"name": "package-name-1"}'],
['file:///project/tsconfig.json', '{ "compilerOptions": { "typeRoots": ["../types"]} }'],
['file:///project/file.ts', 'console.log(GLOBALCONSTANT);'],
['file:///types/types.d.ts', 'declare var GLOBALCONSTANT=1;']

]));
const updater = new FileSystemUpdater(localfs, memfs);
projectManager = new ProjectManager('/', memfs, updater, true);
});
it('loads files from typeRoots', async () => {
await projectManager.ensureReferencedFiles('file:///project/file.ts').toPromise();
memfs.getContent('file:///project/file.ts');
memfs.getContent('file:///types/types.d.ts');
});
});

describe('getPackageName()', () => {
beforeEach(async () => {
memfs = new InMemoryFileSystem('/');
Expand Down