From b2e8c39297ce01479d2548fa1d69a76b784bfaf7 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 28 May 2025 15:27:54 -0700 Subject: [PATCH 1/6] Starting glob fixup --- internal/project/project.go | 9 ++------- internal/project/watch.go | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/internal/project/project.go b/internal/project/project.go index 7820b6658d..854579ae39 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -3,7 +3,6 @@ package project import ( "context" "fmt" - "maps" "slices" "strings" "sync" @@ -156,12 +155,8 @@ func NewProject(name string, kind Kind, currentDirectory string, host ProjectHos } client := host.Client() if host.IsWatchEnabled() && client != nil { - project.failedLookupsWatch = newWatchedFiles(client, lsproto.WatchKindCreate, func(data map[tspath.Path]string) []string { - return slices.Sorted(maps.Values(data)) - }) - project.affectingLocationsWatch = newWatchedFiles(client, lsproto.WatchKindChange|lsproto.WatchKindCreate|lsproto.WatchKindDelete, func(data map[tspath.Path]string) []string { - return slices.Sorted(maps.Values(data)) - }) + project.failedLookupsWatch = newWatchedFiles(client, lsproto.WatchKindCreate, getWatchGlobs) + project.affectingLocationsWatch = newWatchedFiles(client, lsproto.WatchKindChange|lsproto.WatchKindCreate|lsproto.WatchKindDelete, getWatchGlobs) } project.markAsDirty() return project diff --git a/internal/project/watch.go b/internal/project/watch.go index adf32a4dbb..9819caae51 100644 --- a/internal/project/watch.go +++ b/internal/project/watch.go @@ -4,7 +4,9 @@ import ( "context" "slices" + "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/lsp/lsproto" + "github.com/microsoft/typescript-go/internal/tspath" ) const ( @@ -60,3 +62,20 @@ func (w *watchedFiles[T]) update(ctx context.Context, newData T) (updated bool, w.watcherID = watcherID return true, nil } + +func getWatchGlobs(data map[tspath.Path]string) []string { + // TODO: this is still not enough + + var dirSet core.Set[string] + for _, fileName := range data { + dirname := tspath.GetDirectoryPath(fileName) + dirSet.Add(dirname) + } + + globs := make([]string, 0, dirSet.Len()) + for dir := range dirSet.Keys() { + globs = append(globs, dir+"/"+fileGlobPattern) + } + slices.Sort(globs) + return globs +} From e5753093bd44d9f64c0ff6df6904d6820296396c Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 28 May 2025 16:43:01 -0700 Subject: [PATCH 2/6] Port getDirectoryToWatchFailedLookupLocation --- internal/project/project.go | 5 +- internal/project/watch.go | 258 ++++++++++++++++++++++++++++++++++-- 2 files changed, 250 insertions(+), 13 deletions(-) diff --git a/internal/project/project.go b/internal/project/project.go index 854579ae39..d1331dd66c 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -155,8 +155,9 @@ func NewProject(name string, kind Kind, currentDirectory string, host ProjectHos } client := host.Client() if host.IsWatchEnabled() && client != nil { - project.failedLookupsWatch = newWatchedFiles(client, lsproto.WatchKindCreate, getWatchGlobs) - project.affectingLocationsWatch = newWatchedFiles(client, lsproto.WatchKindChange|lsproto.WatchKindCreate|lsproto.WatchKindDelete, getWatchGlobs) + globMapper := createGlobMapper(host) + project.failedLookupsWatch = newWatchedFiles(client, lsproto.WatchKindCreate, globMapper) + project.affectingLocationsWatch = newWatchedFiles(client, lsproto.WatchKindChange|lsproto.WatchKindCreate|lsproto.WatchKindDelete, globMapper) } project.markAsDirty() return project diff --git a/internal/project/watch.go b/internal/project/watch.go index 9819caae51..968fc5add9 100644 --- a/internal/project/watch.go +++ b/internal/project/watch.go @@ -2,7 +2,9 @@ package project import ( "context" + "maps" "slices" + "strings" "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/lsp/lsproto" @@ -63,19 +65,253 @@ func (w *watchedFiles[T]) update(ctx context.Context, newData T) (updated bool, return true, nil } -func getWatchGlobs(data map[tspath.Path]string) []string { - // TODO: this is still not enough +func createGlobMapper(host ProjectHost) func(data map[tspath.Path]string) []string { + rootDir := host.GetCurrentDirectory() + rootPath := tspath.ToPath(rootDir, "", host.FS().UseCaseSensitiveFileNames()) + rootPathComponents := tspath.GetPathComponents(string(rootPath), "") + isRootWatchable := canWatchDirectoryOrFile(rootPathComponents) - var dirSet core.Set[string] - for _, fileName := range data { - dirname := tspath.GetDirectoryPath(fileName) - dirSet.Add(dirname) + return func(data map[tspath.Path]string) []string { + var globSet core.Set[string] + + for path, fileName := range data { + w := getDirectoryToWatchFailedLookupLocation( + fileName, + path, + rootDir, + rootPath, + rootPathComponents, + isRootWatchable, + rootDir, + true, + ) + if w == nil { + continue + } + if w.nonRecursive { + globSet.Add(w.dir + "/" + fileGlobPattern) + } else { + globSet.Add(w.dir + "/" + recursiveFileGlobPattern) + } + } + + globs := slices.AppendSeq(make([]string, 0, globSet.Len()), maps.Keys(globSet.M)) + slices.Sort(globs) + return globs + } +} + +type directoryOfFailedLookupWatch struct { + dir string + dirPath tspath.Path + nonRecursive bool + packageDir *string + packageDirPath *tspath.Path +} + +func getDirectoryToWatchFailedLookupLocation( + failedLookupLocation string, + failedLookupLocationPath tspath.Path, + rootDir string, + rootPath tspath.Path, + rootPathComponents []string, + isRootWatchable bool, + currentDirectory string, + preferNonRecursiveWatch bool, +) *directoryOfFailedLookupWatch { + failedLookupPathComponents := tspath.GetPathComponents(string(failedLookupLocationPath), "") + // Ensure failed look up is normalized path + // !!! needed? + if tspath.IsRootedDiskPath(failedLookupLocation) { + failedLookupLocation = tspath.NormalizePath(failedLookupLocation) + } else { + failedLookupLocation = tspath.GetNormalizedAbsolutePath(failedLookupLocation, currentDirectory) + } + failedLookupComponents := tspath.GetPathComponents(failedLookupLocation, "") + perceivedOsRootLength := perceivedOsRootLengthForWatching(failedLookupPathComponents, len(failedLookupPathComponents)) + if len(failedLookupPathComponents) <= perceivedOsRootLength+1 { + return nil + } + // If directory path contains node module, get the most parent node_modules directory for watching + nodeModulesIndex := slices.Index(failedLookupPathComponents, "node_modules") + if nodeModulesIndex != -1 && nodeModulesIndex+1 <= perceivedOsRootLength+1 { + return nil + } + lastNodeModulesIndex := lastIndex(failedLookupPathComponents, "node_modules") + if isRootWatchable && isInDirectoryPath(rootPathComponents, failedLookupPathComponents) { + if len(failedLookupPathComponents) > len(rootPathComponents)+1 { + // Instead of watching root, watch directory in root to avoid watching excluded directories not needed for module resolution + return getDirectoryOfFailedLookupWatch( + failedLookupComponents, + failedLookupPathComponents, + max(len(rootPathComponents)+1, perceivedOsRootLength+1), + lastNodeModulesIndex, + false, + ) + } else { + // Always watch root directory non recursively + return &directoryOfFailedLookupWatch{ + dir: rootDir, + dirPath: rootPath, + nonRecursive: true, + } + } + } + + return getDirectoryToWatchFromFailedLookupLocationDirectory( + failedLookupComponents, + failedLookupPathComponents, + len(failedLookupPathComponents)-1, + perceivedOsRootLength, + nodeModulesIndex, + rootPathComponents, + lastNodeModulesIndex, + preferNonRecursiveWatch, + ) +} + +func getDirectoryToWatchFromFailedLookupLocationDirectory( + dirComponents []string, + dirPathComponents []string, + dirPathComponentsLength int, + perceivedOsRootLength int, + nodeModulesIndex int, + rootPathComponents []string, + lastNodeModulesIndex int, + preferNonRecursiveWatch bool, +) *directoryOfFailedLookupWatch { + // If directory path contains node module, get the most parent node_modules directory for watching + if nodeModulesIndex != -1 { + // If the directory is node_modules use it to watch, always watch it recursively + return getDirectoryOfFailedLookupWatch( + dirComponents, + dirPathComponents, + nodeModulesIndex+1, + lastNodeModulesIndex, + false, + ) + } + + // Use some ancestor of the root directory + nonRecursive := true + length := dirPathComponentsLength + if !preferNonRecursiveWatch { + for i := range dirPathComponentsLength { + if dirPathComponents[i] != rootPathComponents[i] { + nonRecursive = false + length = max(i+1, perceivedOsRootLength+1) + break + } + } + } + return getDirectoryOfFailedLookupWatch( + dirComponents, + dirPathComponents, + length, + lastNodeModulesIndex, + nonRecursive, + ) +} + +func getDirectoryOfFailedLookupWatch( + dirComponents []string, + dirPathComponents []string, + length int, + lastNodeModulesIndex int, + nonRecursive bool, +) *directoryOfFailedLookupWatch { + packageDirLength := -1 + if lastNodeModulesIndex != -1 && lastNodeModulesIndex+1 >= length && lastNodeModulesIndex+2 < len(dirPathComponents) { + if !strings.HasPrefix(dirPathComponents[lastNodeModulesIndex+1], "@") { + packageDirLength = lastNodeModulesIndex + 2 + } else if lastNodeModulesIndex+3 < len(dirPathComponents) { + packageDirLength = lastNodeModulesIndex + 3 + } + } + var packageDir *string + var packageDirPath *tspath.Path + if packageDirLength != -1 { + packageDir = ptrTo(tspath.GetPathFromPathComponents(dirPathComponents[:packageDirLength])) + packageDirPath = ptrTo(tspath.Path(tspath.GetPathFromPathComponents(dirComponents[:packageDirLength]))) + } + + return &directoryOfFailedLookupWatch{ + dir: tspath.GetPathFromPathComponents(dirComponents[:length]), + dirPath: tspath.Path(tspath.GetPathFromPathComponents(dirPathComponents[:length])), + nonRecursive: nonRecursive, + packageDir: packageDir, + packageDirPath: packageDirPath, } +} - globs := make([]string, 0, dirSet.Len()) - for dir := range dirSet.Keys() { - globs = append(globs, dir+"/"+fileGlobPattern) +func perceivedOsRootLengthForWatching(pathComponents []string, length int) int { + // Ignore "/", "c:/" + if length <= 1 { + return 1 + } + indexAfterOsRoot := 1 + firstComponent := pathComponents[0] + isDosStyle := len(firstComponent) >= 2 && tspath.IsVolumeCharacter(firstComponent[0]) && firstComponent[1] == ':' + if firstComponent != "/" && !isDosStyle && isDosStyleNextPart(pathComponents[1]) { + // ignore "//vda1cs4850/c$/folderAtRoot" + if length == 2 { + return 2 + } + indexAfterOsRoot = 2 + isDosStyle = true } - slices.Sort(globs) - return globs + + afterOsRoot := pathComponents[indexAfterOsRoot] + if isDosStyle && !strings.EqualFold(afterOsRoot, "users") { + // Paths like c:/notUsers + return indexAfterOsRoot + } + + if strings.EqualFold(afterOsRoot, "workspaces") { + // Paths like: /workspaces as codespaces hoist the repos in /workspaces so we have to exempt these from "2" level from root rule + return indexAfterOsRoot + 1 + } + + // Paths like: c:/users/username or /home/username + return indexAfterOsRoot + 2 +} + +func canWatchDirectoryOrFile(pathComponents []string) bool { + length := len(pathComponents) + // Ignore "/", "c:/" + // ignore "/user", "c:/users" or "c:/folderAtRoot" + if length < 2 { + return false + } + perceivedOsRootLength := perceivedOsRootLengthForWatching(pathComponents, length) + return length > perceivedOsRootLength+1 +} + +func isDosStyleNextPart(part string) bool { + return len(part) == 2 && tspath.IsVolumeCharacter(part[0]) && part[1] == '$' +} + +func lastIndex[T comparable](s []T, v T) int { + for i := len(s) - 1; i >= 0; i-- { + if s[i] == v { + return i + } + } + return -1 +} + +func isInDirectoryPath(dirComponents []string, fileOrDirComponents []string) bool { + if len(fileOrDirComponents) < len(dirComponents) { + return false + } + for i := range dirComponents { + if dirComponents[i] != fileOrDirComponents[i] { + return false + } + } + return true +} + +func ptrTo[T any](v T) *T { + return &v } From e81acb69f47dbf568acbaeeb837eefdca69ecebd Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 28 May 2025 16:46:55 -0700 Subject: [PATCH 3/6] Normalize root glob match --- internal/project/project.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/project/project.go b/internal/project/project.go index d1331dd66c..75be8ee20f 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -266,7 +266,7 @@ func (p *Project) getRootFileWatchGlobs() []string { result := make([]string, 0, len(globs)+1) result = append(result, p.configFileName) for dir, recursive := range globs { - result = append(result, fmt.Sprintf("%s/%s", dir, core.IfElse(recursive, recursiveFileGlobPattern, fileGlobPattern))) + result = append(result, fmt.Sprintf("%s/%s", tspath.NormalizePath(dir), core.IfElse(recursive, recursiveFileGlobPattern, fileGlobPattern))) } for _, fileName := range p.parsedCommandLine.LiteralFileNames() { result = append(result, fileName) From a7968022d87fc7b8351743dfa5867c103b23b9f2 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 28 May 2025 17:04:16 -0700 Subject: [PATCH 4/6] Optimize a little --- internal/project/watch.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/internal/project/watch.go b/internal/project/watch.go index 968fc5add9..5ce560a1d0 100644 --- a/internal/project/watch.go +++ b/internal/project/watch.go @@ -2,9 +2,10 @@ package project import ( "context" - "maps" + "fmt" "slices" "strings" + "time" "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/lsp/lsproto" @@ -72,7 +73,14 @@ func createGlobMapper(host ProjectHost) func(data map[tspath.Path]string) []stri isRootWatchable := canWatchDirectoryOrFile(rootPathComponents) return func(data map[tspath.Path]string) []string { - var globSet core.Set[string] + start := time.Now() + + type glob struct { + dir string + nonRecursive bool + } + + var globSet core.Set[glob] for path, fileName := range data { w := getDirectoryToWatchFailedLookupLocation( @@ -88,15 +96,21 @@ func createGlobMapper(host ProjectHost) func(data map[tspath.Path]string) []stri if w == nil { continue } - if w.nonRecursive { - globSet.Add(w.dir + "/" + fileGlobPattern) + globSet.Add(glob{dir: w.dir, nonRecursive: w.nonRecursive}) + } + + globs := make([]string, 0, globSet.Len()) + for g := range globSet.Keys() { + if g.nonRecursive { + globs = append(globs, g.dir+"/"+fileGlobPattern) } else { - globSet.Add(w.dir + "/" + recursiveFileGlobPattern) + globs = append(globs, g.dir+"/"+recursiveFileGlobPattern) } } - - globs := slices.AppendSeq(make([]string, 0, globSet.Len()), maps.Keys(globSet.M)) slices.Sort(globs) + + took := time.Since(start) + host.Log(fmt.Sprintf("createGlobMapper took %s to create %d globs for %d failed lookups", took, len(globs), len(data))) return globs } } From b9808966aac2d5a0bb91dbb326159bbb1a480848 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 28 May 2025 17:22:00 -0700 Subject: [PATCH 5/6] Fix test --- internal/project/service_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/project/service_test.go b/internal/project/service_test.go index 953b37253f..c4533383cc 100644 --- a/internal/project/service_test.go +++ b/internal/project/service_test.go @@ -598,7 +598,7 @@ func TestService(t *testing.T) { // Missing location should be watched assert.Check(t, slices.ContainsFunc(host.ClientMock.WatchFilesCalls()[1].Watchers, func(w *lsproto.FileSystemWatcher) bool { - return *w.GlobPattern.Pattern == "/home/projects/TS/p1/src/z.ts" && *w.Kind == lsproto.WatchKindCreate + return *w.GlobPattern.Pattern == "/home/projects/TS/p1/src/*.{js,jsx,mjs,cjs,ts,tsx,mts,cts,json}" && *w.Kind == lsproto.WatchKindCreate })) // Add a new file through failed lookup watch From c713119b1b5fa21698db043f473e7a86edc069da Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 29 May 2025 08:25:13 -0700 Subject: [PATCH 6/6] PR feedback --- internal/project/watch.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/internal/project/watch.go b/internal/project/watch.go index 5ce560a1d0..2098f0fd82 100644 --- a/internal/project/watch.go +++ b/internal/project/watch.go @@ -7,7 +7,6 @@ import ( "strings" "time" - "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/lsp/lsproto" "github.com/microsoft/typescript-go/internal/tspath" ) @@ -75,12 +74,8 @@ func createGlobMapper(host ProjectHost) func(data map[tspath.Path]string) []stri return func(data map[tspath.Path]string) []string { start := time.Now() - type glob struct { - dir string - nonRecursive bool - } - - var globSet core.Set[glob] + // dir -> recursive + globSet := make(map[string]bool) for path, fileName := range data { w := getDirectoryToWatchFailedLookupLocation( @@ -96,21 +91,21 @@ func createGlobMapper(host ProjectHost) func(data map[tspath.Path]string) []stri if w == nil { continue } - globSet.Add(glob{dir: w.dir, nonRecursive: w.nonRecursive}) + globSet[w.dir] = globSet[w.dir] || !w.nonRecursive } - globs := make([]string, 0, globSet.Len()) - for g := range globSet.Keys() { - if g.nonRecursive { - globs = append(globs, g.dir+"/"+fileGlobPattern) + globs := make([]string, 0, len(globSet)) + for dir, recursive := range globSet { + if recursive { + globs = append(globs, dir+"/"+recursiveFileGlobPattern) } else { - globs = append(globs, g.dir+"/"+recursiveFileGlobPattern) + globs = append(globs, dir+"/"+fileGlobPattern) } } slices.Sort(globs) - took := time.Since(start) - host.Log(fmt.Sprintf("createGlobMapper took %s to create %d globs for %d failed lookups", took, len(globs), len(data))) + timeTaken := time.Since(start) + host.Log(fmt.Sprintf("createGlobMapper took %s to create %d globs for %d failed lookups", timeTaken, len(globs), len(data))) return globs } }