Skip to content

Tweak module resolution failed lookup watching #53420

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

Closed
wants to merge 8 commits into from
Closed

Tweak module resolution failed lookup watching #53420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6c7a47b
No need to handle custom extensions as resolutions are done irrespect…
sheetalkamat Mar 21, 2023
b6bddbf
Update node_modules can be watched option to use node_modules path it…
sheetalkamat Mar 21, 2023
666fe1e
Update src/compiler/resolutionCache.ts
sheetalkamat Mar 22, 2023
bc78220
Merge branch 'main' into extensions
sheetalkamat Mar 22, 2023
1fc3935
Rename
sheetalkamat Mar 22, 2023
137d923
oops didnt use already calculated path
sheetalkamat Mar 23, 2023
d309851
Merge branch 'main' into extensions
sheetalkamat Mar 23, 2023
f64f2ad
Merge branch 'main' into extensions
sheetalkamat Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
61 changes: 17 additions & 44 deletions src/compiler/resolutionCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
Extension,
extensionIsTS,
fileExtensionIs,
fileExtensionIsOneOf,
FileReference,
FileWatcher,
FileWatcherCallback,
Expand Down Expand Up @@ -298,15 +297,6 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
moduleResolutionCache.getPackageJsonInfoCache(),
);

/**
* These are the extensions that failed lookup files will have by default,
* any other extension of failed lookup will be store that path in custom failed lookup path
* This helps in not having to comb through all resolutions when files are added/removed
* Note that .d.ts file also has .d.ts extension hence will be part of default extensions
*/
const failedLookupDefaultExtensions = [Extension.Ts, Extension.Tsx, Extension.Js, Extension.Jsx, Extension.Json];
const customFailedLookupPaths = new Map<string, number>();

const directoryWatchesOfFailedLookups = new Map<string, DirectoryWatchesOfFailedLookup>();
const fileWatchesOfAffectingLocations = new Map<string, FileWatcherOfAffectingLocation>();
const rootDir = rootDirForResolution && removeTrailingDirectorySeparator(getNormalizedAbsolutePath(rootDirForResolution, getCurrentDirectory()));
Expand Down Expand Up @@ -358,7 +348,6 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
function clear() {
clearMap(directoryWatchesOfFailedLookups, closeFileWatcherOf);
clearMap(fileWatchesOfAffectingLocations, closeFileWatcherOf);
customFailedLookupPaths.clear();
nonRelativeExternalModuleResolutions.clear();
closeTypeRootsWatch();
resolvedModuleNames.clear();
Expand Down Expand Up @@ -740,7 +729,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD

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

let nonRecursive = true;
Expand All @@ -763,10 +752,6 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
return canWatchDirectoryOrFile(dirPath) ? { dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath, nonRecursive } : undefined;
}

function isPathWithDefaultFailedLookupExtension(path: Path) {
return fileExtensionIsOneOf(path, failedLookupDefaultExtensions);
}

function watchFailedLookupLocationsOfExternalModuleResolutions<T extends ResolutionWithFailedLookupLocations, R extends ResolutionWithResolvedFileName>(
name: string,
resolution: T,
Expand Down Expand Up @@ -811,12 +796,6 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
const toWatch = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
if (toWatch) {
const { dir, dirPath, nonRecursive } = toWatch;
// If the failed lookup location path is not one of the supported extensions,
// store it in the custom path
if (!isPathWithDefaultFailedLookupExtension(failedLookupLocationPath)) {
const refCount = customFailedLookupPaths.get(failedLookupLocationPath) || 0;
customFailedLookupPaths.set(failedLookupLocationPath, refCount + 1);
}
if (dirPath === rootPath) {
Debug.assert(!nonRecursive);
setAtRoot = true;
Expand Down Expand Up @@ -846,6 +825,12 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
}
}

function isInRootPathOrCanWatchDirectoryOrFile(locationToWatch: string) {
const path = resolutionHost.toPath(locationToWatch);
return isInDirectoryPath(rootPath, path) ||
canWatchDirectoryOrFile(path);
}

function createFileWatcherOfAffectingLocation(affectingLocation: string, forResolution: boolean) {
const fileWatcher = fileWatchesOfAffectingLocations.get(affectingLocation);
if (fileWatcher) {
Expand All @@ -869,7 +854,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
}
const paths = new Set<string>();
paths.add(locationToWatch);
let actualWatcher = canWatchDirectoryOrFile(resolutionHost.toPath(locationToWatch)) ?
let actualWatcher = isInRootPathOrCanWatchDirectoryOrFile(locationToWatch) ?
resolutionHost.watchAffectingFileLocation(locationToWatch, (fileName, eventKind) => {
cachedDirectoryStructureHost?.addOrDeleteFile(fileName, resolutionHost.toPath(locationToWatch), eventKind);
const packageJsonMap = moduleResolutionCache.getPackageJsonInfoCache().getInternalMap();
Expand Down Expand Up @@ -945,17 +930,6 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
const toWatch = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath);
if (toWatch) {
const { dirPath } = toWatch;
const refCount = customFailedLookupPaths.get(failedLookupLocationPath);
if (refCount) {
if (refCount === 1) {
customFailedLookupPaths.delete(failedLookupLocationPath);
}
else {
Debug.assert(refCount > 1);
customFailedLookupPaths.set(failedLookupLocationPath, refCount - 1);
}
}

if (dirPath === rootPath) {
removeAtRoot = true;
}
Expand Down Expand Up @@ -1088,13 +1062,14 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
(startsWithPathChecks ||= new Set()).add(fileOrDirectoryPath);
}
else {
if (!isPathWithDefaultFailedLookupExtension(fileOrDirectoryPath) && !customFailedLookupPaths.has(fileOrDirectoryPath)) {
return false;
}
// Ignore emits from the program
if (isEmittedFileOfProgram(resolutionHost.getCurrentProgram(), fileOrDirectoryPath)) {
return false;
}
// Ignore .map files
if (fileExtensionIs(fileOrDirectoryPath, ".map")) {
return false;
}
// Resolution need to be invalidated if failed lookup location is same as the file or directory getting created
(failedLookupChecks ||= new Set()).add(fileOrDirectoryPath);

Expand Down Expand Up @@ -1222,14 +1197,12 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
}
}

function canWatchTypeRootPath(nodeTypesDirectory: string) {
function canWatchTypeRootPath(typeRoot: string) {
// If type roots is specified, watch that path
if (resolutionHost.getCompilationSettings().typeRoots) return true;

// Otherwise can watch directory only if we can watch the parent directory of node_modules/@types
const dir = getDirectoryPath(getDirectoryPath(nodeTypesDirectory));
const dirPath = resolutionHost.toPath(dir);
return dirPath === rootPath || canWatchDirectoryOrFile(dirPath);
return !!resolutionHost.getCompilationSettings().typeRoots ||
// Otherwise we'll only watch this path if it falls within `rootDir` or
// the path is not disqualified by other criteria ("not `C:\Users\Name\Dir`").
isInRootPathOrCanWatchDirectoryOrFile(getDirectoryPath(typeRoot));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/tscWatch/projectsWithReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ X;`,
});
changeCompilerOpitonsPaths(sys, getTsBuildProjectFilePath("transitiveReferences", "c/tsconfig.json"), { "@ref/*": ["../nrefs/*"] });
},
timeouts: sys => sys.checkTimeoutQueueLengthAndRun(1)
timeouts: sys => sys.runQueuedTimeoutCallbacks()
},
{
caption: "Revert config file edit",
Expand Down Expand Up @@ -371,7 +371,7 @@ X;`,
});
changeCompilerOpitonsPaths(sys, getTsBuildProjectFilePath("transitiveReferences", "c/tsconfig.json"), { "@ref/*": ["../nrefs/*"] });
},
timeouts: sys => sys.checkTimeoutQueueLengthAndRun(1)
timeouts: sys => sys.runQueuedTimeoutCallbacks()
},
{
caption: "Revert config file edit",
Expand Down
8 changes: 4 additions & 4 deletions src/testRunner/unittests/tsserver/projectsWithReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class A {}`
host.ensureFileOrFolder(nRefsTs);
cTsConfigJson.compilerOptions.paths = { "@ref/*": ["../nrefs/*"] };
host.writeFile(cConfig.path, JSON.stringify(cTsConfigJson));
host.checkTimeoutQueueLengthAndRun(2);
host.runQueuedTimeoutCallbacks();

// revert the edit on config file
host.writeFile(cConfig.path, cConfig.content);
Expand All @@ -133,7 +133,7 @@ export class A {}`
host.ensureFileOrFolder(nRefsTs);
bTsConfigJson.compilerOptions.paths = { "@ref/*": ["../nrefs/*"] };
host.writeFile(bConfig.path, JSON.stringify(bTsConfigJson));
host.checkTimeoutQueueLengthAndRun(2);
host.runQueuedTimeoutCallbacks();

// revert the edit on config file
host.writeFile(bConfig.path, bConfig.content);
Expand Down Expand Up @@ -230,7 +230,7 @@ export class A {}`
host.ensureFileOrFolder(nRefsTs);
cTsConfigJson.compilerOptions.paths = { "@ref/*": ["../nrefs/*"] };
host.writeFile(cConfig.path, JSON.stringify(cTsConfigJson));
host.checkTimeoutQueueLengthAndRun(2);
host.runQueuedTimeoutCallbacks();

// revert the edit on config file
host.writeFile(cConfig.path, cConfig.content);
Expand All @@ -248,7 +248,7 @@ export class A {}`
host.ensureFileOrFolder(nRefsTs);
bTsConfigJson.compilerOptions.paths = { "@ref/*": ["../nrefs/*"] };
host.writeFile(bConfig.path, JSON.stringify(bTsConfigJson));
host.checkTimeoutQueueLengthAndRun(2);
host.runQueuedTimeoutCallbacks();

// revert the edit on config file
host.writeFile(bConfig.path, bConfig.content);
Expand Down
4 changes: 4 additions & 0 deletions ...onsoleClearing/when-preserveWatchOutput-is-true-in-config-file/createWatchOfConfigFile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ Shape signatures in builder refreshed for::
/f.ts (used version)
/a/lib/lib.d.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

Comment on lines +47 to +50
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why these were missing before and are added now? I don’t quite see why from the implementation change.

Copy link
Member Author

@sheetalkamat sheetalkamat Mar 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry i thought this was also handled by "canWatchFileOrDirectory" thing but we had logic for watching node_modules/@types at tsconfig or jsconfig directory as well which has some issues when it is /. I will look more into this and fix any other issues with config at root directory.

FsWatches::
/tsconfig.json: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions ...ateWatchProgram-is-invoked-with-configFileParseResult-on-WatchCompilerHostOfConfigFile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ Shape signatures in builder refreshed for::
/f.ts (used version)
/a/lib/lib.d.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/tsconfig.json: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/tscWatch/consoleClearing/with---diagnostics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/f.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/f.ts: *new*
{}
Expand Down
6 changes: 6 additions & 0 deletions tests/baselines/reference/tscWatch/consoleClearing/with---extendedDiagnostics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ CreatingProgramWith::
options: {"watch":true,"extendedDiagnostics":true}
FileWatcher:: Added:: WatchInfo: /f.ts 250 undefined Source file
FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 250 undefined Source file
DirectoryWatcher:: Added:: WatchInfo: /node_modules/@types 1 undefined Type roots
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /node_modules/@types 1 undefined Type roots
[12:00:14 AM] Found 0 errors. Watching for file changes.


Expand All @@ -47,6 +49,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/f.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/f.ts: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/tscWatch/consoleClearing/with---preserveWatchOutput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/f.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/f.ts: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions ...ines/reference/tscWatch/consoleClearing/without---diagnostics-or---extendedDiagnostics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/f.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/f.ts: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions ...scWatch/emit/emit-file-content/elides-const-enums-correctly-in-incremental-compilation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ Shape signatures in builder refreshed for::
/user/someone/projects/myproject/file2.ts (used version)
/user/someone/projects/myproject/file3.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/someone/projects/myproject/file3.ts: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions ...es/reference/tscWatch/emit/emit-file-content/handles-new-lines-carriageReturn-lineFeed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/a/app.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/a/app.ts: *new*
{}
Expand Down
4 changes: 4 additions & 0 deletions tests/baselines/reference/tscWatch/emit/emit-file-content/handles-new-lines-lineFeed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ Shape signatures in builder refreshed for::
/a/lib/lib.d.ts (used version)
/a/app.ts (used version)

PolledWatches::
/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/a/app.ts: *new*
{}
Expand Down
2 changes: 2 additions & 0 deletions ...nlyAffectDirectDependencies/deepImportChanges/errors-for-.d.ts-change-with-incremental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...es/assumeChangesOnlyAffectDirectDependencies/deepImportChanges/errors-for-.d.ts-change.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...sOnlyAffectDirectDependencies/deepImportChanges/errors-for-.ts-change-with-incremental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...ates/assumeChangesOnlyAffectDirectDependencies/deepImportChanges/errors-for-.ts-change.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...Dependencies/file-not-exporting-a-deep-multilevel-import-that-changes-with-incremental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...sOnlyAffectDirectDependencies/file-not-exporting-a-deep-multilevel-import-that-changes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...AffectDirectDependencies/transitive-exports/no-circular-import/export-with-incremental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...assumeChangesOnlyAffectDirectDependencies/transitive-exports/no-circular-import/export.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...fectDirectDependencies/transitive-exports/yes-circular-import/exports-with-incremental.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
2 changes: 2 additions & 0 deletions ...sumeChangesOnlyAffectDirectDependencies/transitive-exports/yes-circular-import/exports.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ Shape signatures in builder refreshed for::
PolledWatches::
/user/username/projects/myproject/node_modules/@types: *new*
{"pollingInterval":500}
/user/username/projects/node_modules/@types: *new*
{"pollingInterval":500}

FsWatches::
/user/username/projects/myproject/tsconfig.json: *new*
Expand Down
Loading