Skip to content

Commit 62372fb

Browse files
jakebaileyjakebailey
committed
Separate ATA inputs from installer outputs
Typings cache writes are outputs of acquisition, not discovery changes, and can arrive in the same flush as their result. Keep those events available to the program while excluding them from ATA invalidation, without losing real package-owned inputs or full invalidations.
1 parent 7c29ddd commit 62372fb

5 files changed

Lines changed: 371 additions & 22 deletions

File tree

‎tsc/internal/project/ata/ata_test.go‎

Lines changed: 303 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package ata_test
22

33
import (
44
"context"
5+
"fmt"
56
"slices"
67
"strings"
78
"sync"
@@ -441,14 +442,314 @@ func TestATA(t *testing.T) {
441442

442443
release()
443444
session.WaitForBackgroundTasks()
445+
snapshot, err = session.APIUpdate(ctx, project.FileChangeSummary{}, nil)
446+
assert.NilError(t, err)
447+
assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil, "the ATA result should be delivered while no inferred project exists")
448+
snapshot.Deref()
449+
session.DidOpenFile(ctx, uri, 1, files["/user/username/projects/project/app.js"].(string), lsproto.LanguageKindJavaScript)
450+
451+
inferred := session.Snapshot().ProjectCollection.InferredProject()
452+
assert.Assert(t, inferred != nil && inferred.GetProgram() != nil)
453+
typingsFile := inferred.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation + "/node_modules/@types/jquery/index.d.ts")
454+
assert.Assert(t, typingsFile != nil, "jquery types should be available immediately after reopening")
455+
})
456+
457+
t.Run("ATA install writes delivered with the result do not discard it", func(t *testing.T) {
458+
t.Parallel()
459+
460+
files := map[string]any{
461+
"/user/username/projects/a/app.js": ``,
462+
"/user/username/projects/a/tsconfig.json": `{"compilerOptions":{"allowJs":true},"typeAcquisition":{"enable":true}}`,
463+
"/user/username/projects/a/package.json": `{"name":"a","dependencies":{"jquery":"^3.1.0"}}`,
464+
"/user/username/projects/b/app.js": ``,
465+
"/user/username/projects/b/tsconfig.json": `{"compilerOptions":{"allowJs":true},"typeAcquisition":{"enable":true}}`,
466+
"/user/username/projects/b/package.json": `{"name":"b","dependencies":{"commander":"^14.0.0"}}`,
467+
}
468+
session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
469+
PackageToFile: map[string]string{
470+
"commander": `declare const commander: { x: number }`,
471+
"jquery": `declare const $: { x: number }`,
472+
},
473+
})
474+
ctx := context.Background()
475+
aURI := lsproto.DocumentUri("file:///user/username/projects/a/app.js")
476+
bURI := lsproto.DocumentUri("file:///user/username/projects/b/app.js")
477+
commanderTypings := projecttestutil.TestTypingsLocation + "/node_modules/@types/commander/index.d.ts"
478+
session.DidOpenFile(ctx, aURI, 1, "", lsproto.LanguageKindJavaScript)
479+
session.WaitForBackgroundTasks()
480+
_, err := session.GetLanguageService(ctx, aURI)
481+
assert.NilError(t, err)
482+
session.WaitForBackgroundTasks()
483+
assert.Assert(t, utils.WatchesFile(strings.ToLower(commanderTypings)), "the typings location should be watched")
484+
485+
session.DidOpenFile(ctx, bURI, 1, "", lsproto.LanguageKindJavaScript)
486+
session.WaitForBackgroundTasks()
487+
session.DidChangeWatchedFiles(ctx, []*lsproto.FileEvent{{
488+
Uri: lsproto.DocumentUri("file://" + commanderTypings),
489+
Type: lsproto.FileChangeTypeCreated,
490+
}})
491+
ls, err := session.GetLanguageService(ctx, bURI)
492+
assert.NilError(t, err)
493+
assert.Assert(t, ls.GetProgram().GetSourceFile(tspath.RootedFilePathFromNormalized(commanderTypings)) != nil, "the install's own writes should not discard its result")
494+
})
495+
496+
t.Run("source edit delivered with an ATA result rejects obsolete typings", func(t *testing.T) {
497+
t.Parallel()
498+
499+
files := map[string]any{
500+
"/user/username/projects/project/app.js": `import "jquery";`,
501+
}
502+
session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
503+
PackageToFile: map[string]string{
504+
"commander": `declare const commander: { x: number }`,
505+
"jquery": `declare const $: { x: number }`,
506+
},
507+
})
508+
originalNpmInstall := utils.NpmExecutor().NpmInstallFunc
509+
jqueryInstallStarted := make(chan struct{}, 1)
510+
releaseJqueryInstall := make(chan struct{})
511+
releaseCommanderInstall := make(chan struct{})
512+
var releaseOnce sync.Once
513+
releaseCommander := func() { releaseOnce.Do(func() { close(releaseCommanderInstall) }) }
514+
defer releaseCommander()
515+
utils.NpmExecutor().NpmInstallFunc = func(ctx context.Context, cwd tspath.RootedDirectoryPath, args []string) ([]byte, error) {
516+
switch {
517+
case slices.Contains(args, "@types/jquery@latest"):
518+
jqueryInstallStarted <- struct{}{}
519+
<-releaseJqueryInstall
520+
case slices.Contains(args, "@types/commander@latest"):
521+
<-releaseCommanderInstall
522+
}
523+
return originalNpmInstall(ctx, cwd, args)
524+
}
525+
ctx := context.Background()
526+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
444527
session.DidOpenFile(ctx, uri, 1, files["/user/username/projects/project/app.js"].(string), lsproto.LanguageKindJavaScript)
528+
waitForInstall(t, jqueryInstallStarted)
529+
close(releaseJqueryInstall)
530+
session.WaitForBackgroundTasks()
445531

532+
session.DidChangeFile(ctx, uri, 2, []lsproto.TextDocumentContentChangePartialOrWholeDocument{{
533+
WholeDocument: &lsproto.TextDocumentContentChangeWholeDocument{Text: `import "commander";`},
534+
}})
446535
ls, err := session.GetLanguageService(ctx, uri)
447536
assert.NilError(t, err)
448-
typingsFile := ls.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation + "/node_modules/@types/jquery/index.d.ts")
449-
assert.Assert(t, typingsFile != nil, "jquery types should be available immediately after reopening")
537+
assert.Assert(t, ls.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/jquery/index.d.ts") == nil)
538+
})
539+
540+
t.Run("package-owned typings deleted with the result invalidate discovery", func(t *testing.T) {
541+
t.Parallel()
542+
543+
const bundledTypes = "/user/username/projects/typings/foo.d.ts"
544+
files := map[string]any{
545+
"/user/username/projects/project/app.js": `import "foo";`,
546+
"/user/username/projects/project/package.json": `{"name":"test","dependencies":{"foo":"^1.0.0"}}`,
547+
"/user/username/projects/project/node_modules/foo/package.json": `{"name":"foo","types":"../../../typings/foo.d.ts"}`,
548+
bundledTypes: `declare const bundledFoo: { x: number }`,
549+
}
550+
session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
551+
PackageToFile: map[string]string{"foo": `declare const foo: { x: number }`},
552+
})
553+
ctx := context.Background()
554+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
555+
session.DidOpenFile(ctx, uri, 1, files["/user/username/projects/project/app.js"].(string), lsproto.LanguageKindJavaScript)
556+
session.WaitForBackgroundTasks()
557+
558+
assert.NilError(t, utils.FS().Remove(bundledTypes))
559+
session.DidChangeWatchedFiles(ctx, []*lsproto.FileEvent{{
560+
Uri: lsproto.DocumentUri("file://" + bundledTypes),
561+
Type: lsproto.FileChangeTypeDeleted,
562+
}})
563+
ls, err := session.GetLanguageService(ctx, uri)
564+
assert.NilError(t, err)
565+
assert.Assert(t, !slices.Contains(ls.GetProgram().CommandLine().FileNames(), bundledTypes))
566+
session.WaitForBackgroundTasks()
567+
ls, err = session.GetLanguageService(ctx, uri)
568+
assert.NilError(t, err)
569+
assert.Assert(t, ls.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/foo/index.d.ts") != nil)
570+
})
571+
572+
t.Run("ATA install for another project does not discard cached inferred typings", func(t *testing.T) {
573+
t.Parallel()
574+
575+
files := map[string]any{
576+
"/user/username/projects/project/app.js": ``,
577+
"/user/username/projects/project/package.json": `{"name":"test","dependencies":{"jquery":"^3.1.0"}}`,
578+
"/user/username/projects/c/app.js": ``,
579+
"/user/username/projects/c/tsconfig.json": `{"compilerOptions":{"allowJs":true},"typeAcquisition":{"enable":true}}`,
580+
"/user/username/projects/c/package.json": `{"name":"c","dependencies":{"commander":"^14.0.0"}}`,
581+
}
582+
session, _ := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
583+
PackageToFile: map[string]string{
584+
"commander": `declare const commander: { x: number }`,
585+
"jquery": `declare const $: { x: number }`,
586+
},
587+
})
588+
ctx := context.Background()
589+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
590+
cURI := lsproto.DocumentUri("file:///user/username/projects/c/app.js")
591+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
592+
session.WaitForBackgroundTasks()
593+
_, err := session.GetLanguageService(ctx, uri)
594+
assert.NilError(t, err)
595+
session.WaitForBackgroundTasks()
596+
session.DidCloseFile(ctx, uri)
597+
snapshot, err := session.APIUpdate(ctx, project.FileChangeSummary{}, nil)
598+
assert.NilError(t, err)
599+
assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil)
600+
snapshot.Deref()
601+
session.WaitForBackgroundTasks()
602+
603+
session.DidOpenFile(ctx, cURI, 1, "", lsproto.LanguageKindJavaScript)
604+
session.WaitForBackgroundTasks()
605+
session.DidChangeWatchedFiles(ctx, []*lsproto.FileEvent{{
606+
Uri: lsproto.DocumentUri("file://" + projecttestutil.TestTypingsLocation + "/node_modules/@types/commander/index.d.ts"),
607+
Type: lsproto.FileChangeTypeCreated,
608+
}})
609+
_, err = session.GetLanguageService(ctx, cURI)
610+
assert.NilError(t, err)
611+
session.WaitForBackgroundTasks()
612+
613+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
614+
inferred := session.Snapshot().ProjectCollection.InferredProject()
615+
assert.Assert(t, inferred != nil && inferred.GetProgram() != nil)
616+
assert.Assert(t, inferred.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/jquery/index.d.ts") != nil)
617+
})
618+
619+
t.Run("excessive typings cache writes do not discard cached inferred typings", func(t *testing.T) {
620+
t.Parallel()
621+
622+
files := map[string]any{
623+
"/user/username/projects/project/app.js": ``,
624+
"/user/username/projects/project/package.json": `{"name":"test","dependencies":{"jquery":"^3.1.0"}}`,
625+
}
626+
session, _ := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
627+
PackageToFile: map[string]string{"jquery": `declare const $: { x: number }`},
628+
})
629+
ctx := context.Background()
630+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
631+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
632+
session.WaitForBackgroundTasks()
633+
_, err := session.GetLanguageService(ctx, uri)
634+
assert.NilError(t, err)
635+
session.WaitForBackgroundTasks()
636+
session.DidCloseFile(ctx, uri)
637+
snapshot, err := session.APIUpdate(ctx, project.FileChangeSummary{}, nil)
638+
assert.NilError(t, err)
639+
assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil)
640+
snapshot.Deref()
641+
642+
events := make([]*lsproto.FileEvent, 1001)
643+
for i := range events {
644+
events[i] = &lsproto.FileEvent{
645+
Uri: lsproto.DocumentUri(fmt.Sprintf("file://%s/node_modules/@types/generated/%d.d.ts", projecttestutil.TestTypingsLocation, i)),
646+
Type: lsproto.FileChangeTypeCreated,
647+
}
648+
}
649+
session.DidChangeWatchedFiles(ctx, events)
650+
session.WaitForBackgroundTasks()
651+
652+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
653+
inferred := session.Snapshot().ProjectCollection.InferredProject()
654+
assert.Assert(t, inferred != nil && inferred.GetProgram() != nil)
655+
assert.Assert(t, inferred.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/jquery/index.d.ts") != nil)
450656
})
451657

658+
t.Run("excessive cache writes do not hide filesystem replacement invalidation", func(t *testing.T) {
659+
t.Parallel()
660+
661+
files := map[string]any{
662+
"/user/username/projects/project/app.js": ``,
663+
"/user/username/projects/project/package.json": `{"name":"test","dependencies":{"jquery":"^3.1.0"}}`,
664+
}
665+
session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
666+
PackageToFile: map[string]string{"jquery": `declare const $: { x: number }`},
667+
})
668+
ctx := context.Background()
669+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
670+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
671+
session.WaitForBackgroundTasks()
672+
_, err := session.GetLanguageService(ctx, uri)
673+
assert.NilError(t, err)
674+
session.WaitForBackgroundTasks()
675+
session.DidCloseFile(ctx, uri)
676+
snapshot, err := session.APIUpdate(ctx, project.FileChangeSummary{}, nil)
677+
assert.NilError(t, err)
678+
assert.Assert(t, snapshot.ProjectCollection.InferredProject() == nil)
679+
snapshot.Deref()
680+
681+
summary := project.FileChangeSummary{}
682+
for i := range 1001 {
683+
summary.Created.Add(lsproto.DocumentUri(fmt.Sprintf("file://%s/node_modules/@types/generated/%d.d.ts", projecttestutil.TestTypingsLocation, i)))
684+
}
685+
replacement := bundled.WrapFS(vfstest.FromMap(map[string]string{
686+
"/user/username/projects/project/app.js": "",
687+
"/user/username/projects/project/package.json": `{"name":"test"}`,
688+
projecttestutil.TestTypingsLocation + "/node_modules/@types/jquery/index.d.ts": `declare const $: { x: number }`,
689+
}, tspath.CaseInsensitive))
690+
snapshot, err = session.APIUpdate(ctx, summary, &project.APISnapshotRequest{
691+
FileSystem: replacement,
692+
ReplaceFileSystem: true,
693+
})
694+
assert.NilError(t, err)
695+
snapshot.Deref()
696+
697+
snapshot, err = session.APIUpdate(ctx, project.FileChangeSummary{}, &project.APISnapshotRequest{
698+
FileSystem: replacement,
699+
OpenFiles: map[tspath.PathKey]tspath.RootedFilePath{
700+
utils.FS().CaseSensitivity().PathKey(uri.FileName().AsPath()): uri.FileName(),
701+
},
702+
})
703+
assert.NilError(t, err)
704+
defer snapshot.Deref()
705+
inferred := snapshot.ProjectCollection.InferredProject()
706+
assert.Assert(t, inferred != nil && inferred.GetProgram() != nil)
707+
assert.Assert(t, inferred.GetProgram().GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/jquery/index.d.ts") == nil)
708+
})
709+
710+
for _, configured := range []bool{false, true} {
711+
name := "inferred"
712+
if configured {
713+
name = "configured"
714+
}
715+
t.Run("typings added by a manifest change reach the "+name+" program", func(t *testing.T) {
716+
t.Parallel()
717+
718+
files := map[string]any{
719+
"/user/username/projects/project/app.js": ``,
720+
"/user/username/projects/project/package.json": `{"name":"test","dependencies":{"jquery":"^3.1.0"}}`,
721+
}
722+
if configured {
723+
files["/user/username/projects/project/tsconfig.json"] = `{"compilerOptions":{"allowJs":true},"typeAcquisition":{"enable":true}}`
724+
}
725+
session, utils := projecttestutil.SetupWithTypingsInstaller(files, &projecttestutil.TypingsInstallerOptions{
726+
PackageToFile: map[string]string{
727+
"commander": `declare const commander: { x: number }`,
728+
"jquery": `declare const $: { x: number }`,
729+
},
730+
})
731+
ctx := context.Background()
732+
uri := lsproto.DocumentUri("file:///user/username/projects/project/app.js")
733+
packageJSONURI := lsproto.DocumentUri("file:///user/username/projects/project/package.json")
734+
session.DidOpenFile(ctx, uri, 1, "", lsproto.LanguageKindJavaScript)
735+
session.WaitForBackgroundTasks()
736+
_, err := session.GetLanguageService(ctx, uri)
737+
assert.NilError(t, err)
738+
session.WaitForBackgroundTasks()
739+
740+
assert.NilError(t, utils.FS().WriteFile(packageJSONURI.FileName(), `{"name":"test","dependencies":{"commander":"^14.0.0","jquery":"^3.1.0"}}`))
741+
session.DidChangeWatchedFiles(ctx, []*lsproto.FileEvent{{Uri: packageJSONURI, Type: lsproto.FileChangeTypeChanged}})
742+
_, err = session.GetLanguageService(ctx, uri)
743+
assert.NilError(t, err)
744+
session.WaitForBackgroundTasks()
745+
ls, err := session.GetLanguageService(ctx, uri)
746+
assert.NilError(t, err)
747+
program := ls.GetProgram()
748+
assert.Assert(t, program.GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/jquery/index.d.ts") != nil)
749+
assert.Assert(t, program.GetSourceFile(projecttestutil.TestTypingsLocation+"/node_modules/@types/commander/index.d.ts") != nil)
750+
})
751+
}
752+
452753
t.Run("inferred project rejects ATA result invalidated after closing last file", func(t *testing.T) {
453754
t.Parallel()
454755

‎tsc/internal/project/filechange.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package project
33
import (
44
"github.com/microsoft/TypeScript/tsc/internal/collections"
55
"github.com/microsoft/TypeScript/tsc/internal/lsp/lsproto"
6+
"github.com/microsoft/TypeScript/tsc/internal/tspath"
67
)
78

89
const excessiveChangeThreshold = 1000
@@ -96,3 +97,25 @@ func mergeFileChangeSummary(dst *FileChangeSummary, src FileChangeSummary) {
9697
dst.IncludesWatchChangeOutsideNodeModules = true
9798
}
9899
}
100+
101+
func (f FileChangeSummary) withoutChangesWithin(directory tspath.RootedPath, caseSensitivity tspath.CaseSensitivity) FileChangeSummary {
102+
if directory == "" {
103+
return f
104+
}
105+
filter := func(uris collections.Set[lsproto.DocumentUri]) collections.Set[lsproto.DocumentUri] {
106+
var result collections.Set[lsproto.DocumentUri]
107+
for uri := range uris.Keys() {
108+
fileName := uri.FileName().AsPath()
109+
directoryKey := caseSensitivity.PathKey(directory)
110+
fileNameKey := caseSensitivity.PathKey(fileName)
111+
if directoryKey != fileNameKey && !directoryKey.ContainsPath(fileNameKey) {
112+
result.Add(uri)
113+
}
114+
}
115+
return result
116+
}
117+
f.Changed = filter(f.Changed)
118+
f.Created = filter(f.Created)
119+
f.Deleted = filter(f.Deleted)
120+
return f
121+
}

‎tsc/internal/project/project.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func (s *inferredProjectATAState) apply(project *Project) {
256256
project.installedTypingsInfo = s.installedTypingsInfo
257257
project.installedTypingsFileNames = slices.Clone(s.installedTypingsFileNames)
258258
project.installedTypingsFilesToWatch = slices.Clone(s.installedTypingsFilesToWatch)
259-
project.typingsFiles = slices.Clone(s.typingsFiles)
259+
project.setTypingsFiles(slices.Clone(s.typingsFiles))
260260
project.typingsWatch = s.typingsWatch
261261
project.installedTypingsSnapshotID = s.snapshotID
262262
if typingsFilesChanged {
@@ -380,7 +380,7 @@ func NewProject(
380380
logger *logging.LogTree,
381381
) *Project {
382382
if logger != nil {
383-
logger.Log(fmt.Sprintf("Creating %sProject: %s, projectDirectory: %s", kind.String(), id, projectDirectory))
383+
logger.Log(fmt.Sprintf("Creating %sProject: %s, currentDirectory: %s", kind.String(), id, projectDirectory))
384384
}
385385
project := &Project{
386386
Kind: kind,
@@ -558,6 +558,14 @@ func (p *Project) SetCommandLine(commandLine *tsoptions.ParsedCommandLine) {
558558
p.dirtyFilePath = ""
559559
}
560560

561+
func (p *Project) setTypingsFiles(typingsFiles []tspath.RootedFilePath) {
562+
if !slices.Equal(p.typingsFiles, typingsFiles) {
563+
p.commandLineWithTypingsFiles = nil
564+
p.commandLineWithTypingsFilesOnce = sync.Once{}
565+
}
566+
p.typingsFiles = typingsFiles
567+
}
568+
561569
// getCommandLineWithTypingsFiles returns the command line augmented with typing files if ATA is enabled.
562570
func (p *Project) getCommandLineWithTypingsFiles() *tsoptions.ParsedCommandLine {
563571
if len(p.typingsFiles) == 0 {

0 commit comments

Comments
 (0)