Skip to content

Do not watch parent folders recursively if not needed #24933

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.

Sign up for GitHub

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

Merged
sheetalkamat merged 2 commits into from
Jun 14, 2018
Merged

Do not watch parent folders recursively if not needed #24933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46e7cca
Test for amd resolution setting the recursive directory watcher in th…
sheetalkamat Jun 13, 2018
cd97adb
Do not watch the parent folders recursively if not needed.
sheetalkamat Jun 13, 2018
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
25 changes: 16 additions & 9 deletions src/compiler/resolutionCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ namespace ts {
watcher: FileWatcher;
/** ref count keeping this directory watch alive */
refCount: number;
/** is the directory watched being non recursive */
nonRecursive?: boolean;
}

interface DirectoryOfFailedLookupWatch {
dir: string;
dirPath: Path;
nonRecursive?: boolean;
ignore?: true;
}

Expand Down Expand Up @@ -251,7 +254,6 @@ namespace ts {
perDirectoryResolution = createMap();
perDirectoryCache.set(dirPath, perDirectoryResolution);
}

const resolvedModules: R[] = [];
const compilerOptions = resolutionHost.getCompilationSettings();
const hasInvalidatedNonRelativeUnresolvedImport = logChanges && isFileWithInvalidatedNonRelativeUnresolvedImports(path);
Expand Down Expand Up @@ -393,6 +395,7 @@ namespace ts {

function getDirectoryToWatchFailedLookupLocation(failedLookupLocation: string, failedLookupLocationPath: Path): DirectoryOfFailedLookupWatch {
if (isInDirectoryPath(rootPath, failedLookupLocationPath)) {
// Always watch root directory recursively
return { dir: rootDir!, dirPath: rootPath }; // TODO: GH#18217
}

Expand All @@ -409,11 +412,12 @@ namespace ts {
dirPath = getDirectoryPath(dirPath);
}

// If the directory is node_modules use it to watch
// If the directory is node_modules use it to watch, always watch it recursively
if (isNodeModulesDirectory(dirPath)) {
return filterFSRootDirectoriesToWatch({ dir, dirPath }, getDirectoryPath(dirPath));
}

let nonRecursive = true;
// Use some ancestor of the root directory
let subDirectoryPath: Path | undefined, subDirectory: string | undefined;
if (rootPath !== undefined) {
Expand All @@ -422,14 +426,15 @@ namespace ts {
if (parentPath === dirPath) {
break;
}
nonRecursive = false;
subDirectoryPath = dirPath;
subDirectory = dir;
dirPath = parentPath;
dir = getDirectoryPath(dir);
}
}

return filterFSRootDirectoriesToWatch({ dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath }, dirPath);
return filterFSRootDirectoriesToWatch({ dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath, nonRecursive }, dirPath);
}

function isPathWithDefaultFailedLookupExtension(path: Path) {
Expand All @@ -452,7 +457,7 @@ namespace ts {
let setAtRoot = false;
for (const failedLookupLocation of failedLookupLocations) {
const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation);
const { dir, dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
const { dir, dirPath, nonRecursive, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
if (!ignore) {
// If the failed lookup location path is not one of the supported extensions,
// store it in the custom path
Expand All @@ -464,23 +469,25 @@ namespace ts {
setAtRoot = true;
}
else {
setDirectoryWatcher(dir, dirPath);
setDirectoryWatcher(dir, dirPath, nonRecursive);
}
}
}

if (setAtRoot) {
// This is always recursive
setDirectoryWatcher(rootDir!, rootPath); // TODO: GH#18217
}
}

function setDirectoryWatcher(dir: string, dirPath: Path) {
function setDirectoryWatcher(dir: string, dirPath: Path, nonRecursive?: boolean) {
const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath);
if (dirWatcher) {
Debug.assert(!!nonRecursive === !!dirWatcher.nonRecursive);
dirWatcher.refCount++;
}
else {
directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 });
directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath, nonRecursive), refCount: 1, nonRecursive });
}
}

Expand Down Expand Up @@ -530,7 +537,7 @@ namespace ts {
dirWatcher.refCount--;
}

function createDirectoryWatcher(directory: string, dirPath: Path) {
function createDirectoryWatcher(directory: string, dirPath: Path, nonRecursive: boolean | undefined) {
return resolutionHost.watchDirectoryOfFailedLookupLocation(directory, fileOrDirectory => {
const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory);
if (cachedDirectoryStructureHost) {
Expand All @@ -541,7 +548,7 @@ namespace ts {
if (!allFilesHaveInvalidatedResolution && invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) {
resolutionHost.onInvalidatedResolution();
}
}, WatchDirectoryFlags.Recursive);
}, nonRecursive ? WatchDirectoryFlags.None : WatchDirectoryFlags.Recursive);
}

function removeResolutionsOfFileFromCache(cache: Map<Map<ResolutionWithFailedLookupLocations>>, filePath: Path) {
Expand Down
72 changes: 66 additions & 6 deletions src/testRunner/unittests/tsserverProjectSystem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8011,10 +8011,10 @@ new C();`
checkCompleteEvent(session, 2, expectedSequenceId);
}

function verifyWatchedFilesAndDirectories(host: TestServerHost, files: string[], directories: string[]) {
function verifyWatchedFilesAndDirectories(host: TestServerHost, files: string[], recursiveDirectories: string[], nonRecursiveDirectories: string[]) {
checkWatchedFilesDetailed(host, files.filter(f => f !== recognizersDateTimeSrcFile.path), 1);
checkWatchedDirectories(host, emptyArray, /*recursive*/ false);
checkWatchedDirectoriesDetailed(host, directories, 1, /*recursive*/ true);
checkWatchedDirectoriesDetailed(host, nonRecursiveDirectories, 1, /*recursive*/ false);
checkWatchedDirectoriesDetailed(host, recursiveDirectories, 1, /*recursive*/ true);
}

function createSessionAndOpenFile(host: TestServerHost) {
Expand All @@ -8035,22 +8035,23 @@ new C();`
const filesWithNodeModulesSetup = [...filesWithSources, nodeModulesRecorgnizersText];
const filesAfterCompilation = [...filesWithNodeModulesSetup, recongnizerTextDistTypingFile];

const watchedDirectoriesWithResolvedModule = [`${recognizersDateTime}/src`, withPathMapping ? packages : recognizersDateTime, ...getTypeRootsFromLocation(recognizersDateTime)];
const watchedDirectoriesWithResolvedModule = [`${recognizersDateTime}/src`, ...(withPathMapping ? emptyArray : [recognizersDateTime]), ...getTypeRootsFromLocation(recognizersDateTime)];
const watchedDirectoriesWithUnresolvedModule = [recognizersDateTime, ...(withPathMapping ? [recognizersText] : emptyArray), ...watchedDirectoriesWithResolvedModule, ...getNodeModuleDirectories(packages)];
const nonRecursiveWatchedDirectories = withPathMapping ? [packages] : emptyArray;

function verifyProjectWithResolvedModule(session: TestSession) {
const projectService = session.getProjectService();
const project = projectService.configuredProjects.get(recognizerDateTimeTsconfigPath)!;
checkProjectActualFiles(project, filesInProjectWithResolvedModule);
verifyWatchedFilesAndDirectories(session.host, filesInProjectWithResolvedModule, watchedDirectoriesWithResolvedModule);
verifyWatchedFilesAndDirectories(session.host, filesInProjectWithResolvedModule, watchedDirectoriesWithResolvedModule, nonRecursiveWatchedDirectories);
verifyErrors(session, []);
}

function verifyProjectWithUnresolvedModule(session: TestSession) {
const projectService = session.getProjectService();
const project = projectService.configuredProjects.get(recognizerDateTimeTsconfigPath)!;
checkProjectActualFiles(project, filesInProjectWithUnresolvedModule);
verifyWatchedFilesAndDirectories(session.host, filesInProjectWithUnresolvedModule, watchedDirectoriesWithUnresolvedModule);
verifyWatchedFilesAndDirectories(session.host, filesInProjectWithUnresolvedModule, watchedDirectoriesWithUnresolvedModule, nonRecursiveWatchedDirectories);
const startOffset = recognizersDateTimeSrcFile.content.indexOf('"') + 1;
verifyErrors(session, [
createDiagnostic({ line: 1, offset: startOffset }, { line: 1, offset: startOffset + moduleNameInFile.length }, Diagnostics.Cannot_find_module_0, [moduleName])
Expand Down Expand Up @@ -8506,6 +8507,65 @@ new C();`
}
});
});

it("when watching directories for failed lookup locations in amd resolution", () => {
const projectRoot = "/user/username/projects/project";
const nodeFile: File = {
path: `${projectRoot}/src/typings/node.d.ts`,
content: `
declare module "fs" {
export interface something {
}
}`
};
const electronFile: File = {
path: `${projectRoot}/src/typings/electron.d.ts`,
content: `
declare module 'original-fs' {
import * as fs from 'fs';
export = fs;
}`
};
const srcFile: File = {
path: `${projectRoot}/src/somefolder/srcfile.ts`,
content: `
import { x } from "somefolder/module1";
import { x } from "somefolder/module2";
const y = x;`
};
const moduleFile: File = {
path: `${projectRoot}/src/somefolder/module1.ts`,
content: `
export const x = 10;`
};
const configFile: File = {
path: `${projectRoot}/src/tsconfig.json`,
content: JSON.stringify({
compilerOptions: {
module: "amd",
moduleResolution: "classic",
target: "es5",
outDir: "../out",
baseUrl: "./",
typeRoots: ["typings"]

}
})
};
const files = [nodeFile, electronFile, srcFile, moduleFile, configFile, libFile];
const host = createServerHost(files);
const service = createProjectService(host);
service.openClientFile(srcFile.path, srcFile.content, ScriptKind.TS, projectRoot);
checkProjectActualFiles(service.configuredProjects.get(configFile.path)!, files.map(f => f.path));
checkWatchedFilesDetailed(host, mapDefined(files, f => f === srcFile ? undefined : f.path), 1);
checkWatchedDirectoriesDetailed(host, [`${projectRoot}`], 1, /*recursive*/ false); // failed lookup for fs
const expectedWatchedDirectories = createMap<number>();
expectedWatchedDirectories.set(`${projectRoot}/src`, 2); // Wild card and failed lookup
expectedWatchedDirectories.set(`${projectRoot}/somefolder`, 1); // failed lookup for somefolder/module2
expectedWatchedDirectories.set(`${projectRoot}/node_modules`, 1); // failed lookup for with node_modules/@types/fs
expectedWatchedDirectories.set(`${projectRoot}/src/typings`, 1); // typeroot directory
checkWatchedDirectoriesDetailed(host, expectedWatchedDirectories, /*recursive*/ true);
});
});

describe("tsserverProjectSystem watchDirectories implementation", () => {
Expand Down