[5.6-beta] #57196 Searching ancestor and its references for default projects regresses TSServer performance for orphaned files

[MichaelMitchell-at]

🔎 Search Terms

Search ancestor and its references for default projects orphan solution

🕗 Version & Regression Information

• This changed in commit or PR Search ancestor and its references for default projects #57196

⏯ Playground Link

No response

💻 Code

No response

🙁 Actual behavior

Context: We have a monorepo with thousands of projects. Roughly, the layout of the codebase follows this structure:

• tsconfig.json - "solution style" tsconfig that references all other tsconfig.json files
• folderA
  • *.tsx files
  • tsconfig.json - "solution style" tsconfig that points to the sibling tsconfig.non_test.json and tsconfig.test.json files
  • tsconfig.non_test.json - Includes all non-test source files in the current directory, excluding ones that are in projects in sub-directories, such as in subSubFolderA.
  • tsconfig.test.json - Includes all test source files in the current directory, excluding ones that are in projects in sub-directories, such as in subSubFolderA.
    • subFolderA
      • *.tsx files
      • subSubFolderA
        • *.tsx files
        • tsconfig.json
        • tsconfig.non_test.json
        • tsconfig.test.json
• folderB
  • tsconfig.json
  • tsconfig.non_test.json
  • tsconfig.test.json

We used to have to maintain these tsconfig files by hand, but now they are all generated using our tooling built around Bazel which can do intelligent things like infer what project references a project needs by looking at the imports in its source files.

Anyway, the reason I described all this is to make it easier to see how for us it's a common workflow when creating a new project to create new files and then generate the tsconfig after the fact. The period before the tsconfig file is generated, all of the new files are treated as orphans. When any of these files are opened in VSCode, TSServer will search all the way up the project tree, looking at all the solution tsconfigs along the way, and loading all of the projects referenced. This eventually reaches the tsconfig.json in the root, which leads to all projects being loaded into the project graph. At this point TSServer will eventually creep up past the 48 GB memory limit we've generously given it until it crashes.

I spent many hours last week debugging what was happening (literally attaching a debugger to tsserver.js and stepping through the code). It was not until seeing the 5.6 beta announcement did I realize that this was a recent change (we have been using nightly versions to access isolatedDeclarations and noCheck). Up until recently, our root tsconfig.json actually had a different name, so it wasn't part of the ancestor search. We named it back to tsconfig.json to reduce confusion for our developers, so I had only ascribed our recent problem to that change.

🙂 Expected behavior

Something that wasn't clear to me during my debugging was why TSServer needed to actually load a project into the graph in order to determine whether it contained a given source file. Maybe there's a good reason for this, but perhaps that's an opportunity for optimization so that a project is only gets loaded if an open file belongs to it or a project that transitively references it.

It would be great to have some way to opt back into the pre-#57196 behavior. This seems like it would be straight-forward by adding a tsconfig option to indicate that if an owner exists for a source file, it won't be in an ancestor. Example:

transitivelyReferencesAllSourcesInRoot (default: false) - Indicates that the current project transitively includes or references all sources in this directory and prevents searching of ancestors.

The actual solution I eventually went with was to patch TSServer so that when it recursively traverses project references, it skips over ones that could not possibly contain the triggering source file, simply by looking to see if the path of the tsconfig was a prefix of the other source file path (our rootDir is always the default). I produced the patch for that below (I don't have a branch to share because yes, I modified the already bundled JS file by hand, instead of working from the original source and building it). It could also be valuable to expose a tsconfig option to simulate this behavior. Example:

disableProjectReferenceSearchingForSourceOutsideRoot (default: false) - do not recursively load a project reference if its rootDir doesn't contain the source file.

Additional information about the issue

Our workaround patch

diff --git a/lib/typescript.js b/lib/typescript.js
index 72e2cceea83e9343cf9266476282b57bdebc236b..8e4ac496d61c0e1f879ec024941cf231fdff9be2 100644
--- a/lib/typescript.js
+++ b/lib/typescript.js
@@ -184850,9 +184850,9 @@ function forEachAncestorProject(info, project, cb, kind, reason, allowDeferredCl
     project = ancestor.project;
   }
 }
-function forEachResolvedProjectReferenceProject(project, fileName, cb, kind, reason, allowDeferredClosed, triggerFile, reloadedProjects) {
+function forEachResolvedProjectReferenceProject(project, fileName, cb, kind, reason, allowDeferredClosed, triggerFile, reloadedProjects, projectReferenceFilter = () => true) {
   var _a;
-  const resolvedRefs = (_a = project.getCurrentProgram()) == null ? void 0 : _a.getResolvedProjectReferences();
+  const resolvedRefs = (_a = project.getCurrentProgram()) == null ? void 0 : _a.getResolvedProjectReferences()?.filter(projectReferenceFilter);
   if (!resolvedRefs) return void 0;
   const possibleDefaultRef = fileName ? project.getResolvedProjectReferenceToRedirect(fileName) : void 0;
   if (possibleDefaultRef) {
@@ -184870,7 +184870,9 @@ function forEachResolvedProjectReferenceProject(project, fileName, cb, kind, rea
         project.getCompilerOptions(),
         (ref, loadKind) => possibleDefaultRef === ref ? callback(ref, loadKind) : void 0,
         kind,
-        project.projectService
+        project.projectService,
+        /*seenResolvedRefs*/ undefined,
+        projectReferenceFilter
       );
       if (result) return result;
     }
@@ -184880,7 +184882,9 @@ function forEachResolvedProjectReferenceProject(project, fileName, cb, kind, rea
     project.getCompilerOptions(),
     (ref, loadKind) => possibleDefaultRef !== ref ? callback(ref, loadKind) : void 0,
     kind,
-    project.projectService
+    project.projectService,
+    /*seenResolvedRefs*/ undefined,
+    projectReferenceFilter
   );
   function callback(ref, loadKind) {
     const result = project.projectService.findCreateOrReloadConfiguredProject(
@@ -184911,7 +184915,7 @@ function forEachResolvedProjectReferenceProject(project, fileName, cb, kind, rea
     if (result) return result;
   }
 }
-function forEachResolvedProjectReferenceProjectWorker(resolvedProjectReferences, parentOptions, cb, kind, projectService, seenResolvedRefs) {
+function forEachResolvedProjectReferenceProjectWorker(resolvedProjectReferences, parentOptions, cb, kind, projectService, seenResolvedRefs, projectReferenceFilter) {
   const loadKind = parentOptions.disableReferencedProjectLoad ? 0 /* Find */ : kind;
   return forEach(resolvedProjectReferences, (ref) => {
     if (!ref) return void 0;
@@ -184926,7 +184930,7 @@ function forEachResolvedProjectReferenceProjectWorker(resolvedProjectReferences,
       return result;
     }
     (seenResolvedRefs || (seenResolvedRefs = /* @__PURE__ */ new Map())).set(canonicalPath, loadKind);
-    return ref.references && forEachResolvedProjectReferenceProjectWorker(ref.references, ref.commandLine.options, cb, loadKind, projectService, seenResolvedRefs);
+    return ref.references && forEachResolvedProjectReferenceProjectWorker(ref.references.filter(projectReferenceFilter), ref.commandLine.options, cb, loadKind, projectService, seenResolvedRefs, projectReferenceFilter);
   });
 }
 function forEachPotentialProjectReference(project, cb) {
@@ -186089,6 +186093,7 @@ var _ProjectService = class _ProjectService {
       }
       const parentPath = asNormalizedPath(getDirectoryPath(searchPath));
       if (parentPath === searchPath) break;
+      if (parentPath === projectRootPath) break;
       searchPath = parentPath;
       searchTsconfig = searchJsconfig = true;
     } while (anySearchPathOk || isSearchPathInProjectRoot());
@@ -187517,7 +187522,15 @@ Dynamic files must always be opened with service's current directory or service
         `Creating project referenced in solution ${project.projectName} to find possible configured project for ${info.fileName} to open`,
         allowDeferredClosed,
         info.fileName,
-        reloadedProjects
+        reloadedProjects,
+        /*projectReferenceFilter*/ (projectRef) => {
+          if (!projectRef) {
+            return true;
+          }
+          const configFileDirectory = toNormalizedPath(getDirectoryPath(projectRef.sourceFile.resolvedPath));
+          const triggerFilePath = toNormalizedPath(info.path);
+          return containsPath(configFileDirectory, triggerFilePath);
+        },
       );
     }
     function tryFindDefaultConfiguredProjectFromAncestor(project) {

[sheetalkamat]

Currently disableSolutionSearching controls solution searching for file open as well as for find all references .. we are looking for feedback on why we would need two flags

We have to load projects because not all files in the program are part of given by projects , some of the d ts files could be opened and are part of the project so there is no guarantee if we found project till we have loaded it ..

[MichaelMitchell-at]

What about something like disableProjectReferenceSearchingForSourceOutsideRoot. That seems to cover a different scenario than disableSolutionSearching.

We have to load projects because not all files in the program are part of given by projects , some of the d ts files could be opened and are part of the project so there is no guarantee if we found project till we have loaded it ..

I don't think I really understand why a project needs to be loaded to determine if it contains a source file. Is that just not just a function of matching the include+exclude+files globs against the source path? And even if a project needs to be loaded, do we need to retain it in memory if we determine it doesn't contain the source and no files are open within that project?

[MichaelMitchell-at]

I saw that #57196 was just reverted in #59634. If there are plans to implement this or a similar behavior again I can leave this issue open, but otherwise perhaps this issue can be closed now?

[jakebailey]

There are plans to reimplement it, but in a way that doesn't hurt performance (or maybe more generally improves performance / reduces unnecessary loads).

[MichaelMitchell-at]

Thanks, in that case I'll leave it up to y'all's judgement whether this should be closed

[sheetalkamat]

#59688 should have fixed this.