-
Notifications
You must be signed in to change notification settings - Fork 720
Invalidate caches on batches of 1000+ watch changes #1869
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.
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
Open
andrewbranch
wants to merge
4
commits into
microsoft:main
Choose a base branch
from
andrewbranch:bulk-watch-changes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
9dc9244
Invalidate file cache after large sets of watch changes
andrewbranch d9140a8
Invalidate config cache on too many changes
andrewbranch 5a7eb44
Treat node_modules-only changes separately so config caches don’t nee…
andrewbranch 1234750
Avoid invalidating anything if present changes have no effect
andrewbranch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,251 @@ | ||
package project_test | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/microsoft/typescript-go/internal/bundled" | ||
"github.com/microsoft/typescript-go/internal/core" | ||
"github.com/microsoft/typescript-go/internal/lsp/lsproto" | ||
"github.com/microsoft/typescript-go/internal/project" | ||
"github.com/microsoft/typescript-go/internal/testutil/projecttestutil" | ||
"gotest.tools/v3/assert" | ||
) | ||
|
||
func TestBulkCacheInvalidation(t *testing.T) { | ||
t.Parallel() | ||
|
||
if !bundled.Embedded { | ||
t.Skip("bundled files are not embedded") | ||
} | ||
|
||
// Base file structure for testing | ||
baseFiles := map[string]any{ | ||
"/project/tsconfig.json": `{ | ||
"compilerOptions": { | ||
"strict": true, | ||
"target": "es2015" | ||
}, | ||
"include": ["src/**/*"] | ||
}`, | ||
"/project/src/index.ts": `import { helper } from "./helper"; console.log(helper);`, | ||
"/project/src/helper.ts": `export const helper = "test";`, | ||
"/project/src/utils/lib.ts": `export function util() { return "util"; }`, | ||
} | ||
|
||
t.Run("large number of node_modules changes invalidates only node_modules cache", func(t *testing.T) { | ||
t.Parallel() | ||
session, utils := projecttestutil.Setup(baseFiles) | ||
|
||
// Open a file to create the project | ||
session.DidOpenFile(context.Background(), "file:///project/src/index.ts", 1, baseFiles["/project/src/index.ts"].(string), lsproto.LanguageKindTypeScript) | ||
|
||
// Get initial snapshot and verify config | ||
ls, err := session.GetLanguageService(context.Background(), "file:///project/src/index.ts") | ||
assert.NilError(t, err) | ||
assert.Equal(t, ls.GetProgram().Options().Target, core.ScriptTargetES2015) | ||
|
||
snapshotBefore, release := session.Snapshot() | ||
defer release() | ||
configBefore := snapshotBefore.ConfigFileRegistry | ||
|
||
// Create excessive changes in node_modules (1001 changes to exceed threshold) | ||
fileEvents := generateFileEvents(1001, "file:///project/node_modules/generated/file%d.js", lsproto.FileChangeTypeCreated) | ||
|
||
// Update tsconfig.json on disk to test that configs don't get reloaded | ||
err = utils.FS().WriteFile("/project/tsconfig.json", `{ | ||
"compilerOptions": { | ||
"strict": true, | ||
"target": "esnext" | ||
}, | ||
"include": ["src/**/*"] | ||
}`, false) | ||
assert.NilError(t, err) | ||
|
||
// Process the excessive node_modules changes | ||
session.DidChangeWatchedFiles(context.Background(), fileEvents) | ||
|
||
// Get language service again to trigger snapshot update | ||
ls, err = session.GetLanguageService(context.Background(), "file:///project/src/index.ts") | ||
assert.NilError(t, err) | ||
|
||
snapshotAfter, release := session.Snapshot() | ||
defer release() | ||
configAfter := snapshotAfter.ConfigFileRegistry | ||
|
||
// Config should NOT have been reloaded (target should remain ES2015, not esnext) | ||
assert.Equal(t, ls.GetProgram().Options().Target, core.ScriptTargetES2015, "Config should not have been reloaded for node_modules-only changes") | ||
|
||
// Config registry should be the same instance (no configs reloaded) | ||
assert.Equal(t, configBefore, configAfter, "Config registry should not have changed for node_modules-only changes") | ||
}) | ||
|
||
t.Run("large number of changes outside node_modules causes config reload", func(t *testing.T) { | ||
t.Parallel() | ||
session, utils := projecttestutil.Setup(baseFiles) | ||
|
||
// Open a file to create the project | ||
session.DidOpenFile(context.Background(), "file:///project/src/index.ts", 1, baseFiles["/project/src/index.ts"].(string), lsproto.LanguageKindTypeScript) | ||
|
||
// Get initial state | ||
ls, err := session.GetLanguageService(context.Background(), "file:///project/src/index.ts") | ||
assert.NilError(t, err) | ||
assert.Equal(t, ls.GetProgram().Options().Target, core.ScriptTargetES2015) | ||
|
||
snapshotBefore, release := session.Snapshot() | ||
defer release() | ||
|
||
// Update tsconfig.json on disk | ||
err = utils.FS().WriteFile("/project/tsconfig.json", `{ | ||
"compilerOptions": { | ||
"strict": true, | ||
"target": "esnext" | ||
}, | ||
"include": ["src/**/*"] | ||
}`, false) | ||
assert.NilError(t, err) | ||
// Add root file | ||
err = utils.FS().WriteFile("/project/src/rootFile.ts", `console.log("root file")`, false) | ||
assert.NilError(t, err) | ||
|
||
// Create excessive changes outside node_modules (1001 changes to exceed threshold) | ||
fileEvents := generateFileEvents(1001, "file:///project/generated/file%d.ts", lsproto.FileChangeTypeCreated) | ||
|
||
// Process the excessive changes outside node_modules | ||
session.DidChangeWatchedFiles(context.Background(), fileEvents) | ||
|
||
// Get language service again to trigger snapshot update | ||
ls, err = session.GetLanguageService(context.Background(), "file:///project/src/index.ts") | ||
assert.NilError(t, err) | ||
|
||
snapshotAfter, release := session.Snapshot() | ||
defer release() | ||
|
||
// Config SHOULD have been reloaded (target should now be esnext and new root file present) | ||
assert.Equal(t, ls.GetProgram().Options().Target, core.ScriptTargetESNext, "Config should have been reloaded for changes outside node_modules") | ||
assert.Check(t, ls.GetProgram().GetSourceFile("/project/src/rootFile.ts") != nil, "New root file should be present") | ||
|
||
// Snapshots should be different | ||
assert.Assert(t, snapshotBefore != snapshotAfter, "Snapshot should have changed after bulk invalidation outside node_modules") | ||
}) | ||
|
||
t.Run("large number of changes outside node_modules causes project reevaluation", func(t *testing.T) { | ||
t.Parallel() | ||
session, utils := projecttestutil.Setup(baseFiles) | ||
|
||
// Open a file that will initially use the root tsconfig | ||
session.DidOpenFile(context.Background(), "file:///project/src/utils/lib.ts", 1, baseFiles["/project/src/utils/lib.ts"].(string), lsproto.LanguageKindTypeScript) | ||
|
||
// Initially, the file should use the root project (strict mode) | ||
snapshot, release := session.Snapshot() | ||
defer release() | ||
initialProject := snapshot.GetDefaultProject("file:///project/src/utils/lib.ts") | ||
assert.Equal(t, initialProject.Name(), "/project/tsconfig.json", "Should initially use root tsconfig") | ||
|
||
// Get language service to verify initial strict mode | ||
ls, err := session.GetLanguageService(context.Background(), "file:///project/src/utils/lib.ts") | ||
assert.NilError(t, err) | ||
assert.Equal(t, ls.GetProgram().Options().Strict, core.TSTrue, "Should initially use strict mode from root config") | ||
|
||
// Now create the nested tsconfig (this would normally be detected, but we'll simulate a missed event) | ||
err = utils.FS().WriteFile("/project/src/utils/tsconfig.json", `{ | ||
"compilerOptions": { | ||
"strict": false, | ||
"target": "esnext" | ||
} | ||
}`, false) | ||
assert.NilError(t, err) | ||
|
||
// Create excessive changes outside node_modules to trigger bulk invalidation | ||
// This simulates the scenario where the nested tsconfig creation was missed in the flood of changes | ||
fileEvents := generateFileEvents(1001, "file:///project/src/generated/file%d.ts", lsproto.FileChangeTypeCreated) | ||
|
||
// Process the excessive changes - this should trigger project reevaluation | ||
session.DidChangeWatchedFiles(context.Background(), fileEvents) | ||
|
||
// Get language service - this should now find the nested config and switch projects | ||
ls, err = session.GetLanguageService(context.Background(), "file:///project/src/utils/lib.ts") | ||
assert.NilError(t, err) | ||
|
||
snapshot, release = session.Snapshot() | ||
defer release() | ||
newProject := snapshot.GetDefaultProject("file:///project/src/utils/lib.ts") | ||
|
||
// The file should now use the nested tsconfig | ||
assert.Equal(t, newProject.Name(), "/project/src/utils/tsconfig.json", "Should now use nested tsconfig after bulk invalidation") | ||
assert.Equal(t, ls.GetProgram().Options().Strict, core.TSFalse, "Should now use non-strict mode from nested config") | ||
assert.Equal(t, ls.GetProgram().Options().Target, core.ScriptTargetESNext, "Should use esnext target from nested config") | ||
}) | ||
|
||
t.Run("excessive changes only in node_modules does not affect config file names cache", func(t *testing.T) { | ||
t.Parallel() | ||
testConfigFileNamesCacheBehavior(t, "file:///project/node_modules/generated/file%d.js", false, "node_modules changes should not clear config cache") | ||
}) | ||
|
||
t.Run("excessive changes outside node_modules clears config file names cache", func(t *testing.T) { | ||
t.Parallel() | ||
testConfigFileNamesCacheBehavior(t, "file:///project/generated/file%d.ts", true, "non-node_modules changes should clear config cache") | ||
}) | ||
} | ||
|
||
// Helper function to generate excessive file change events | ||
func generateFileEvents(count int, pathTemplate string, changeType lsproto.FileChangeType) []*lsproto.FileEvent { | ||
var events []*lsproto.FileEvent | ||
for i := range count { | ||
events = append(events, &lsproto.FileEvent{ | ||
Uri: lsproto.DocumentUri(fmt.Sprintf(pathTemplate, i)), | ||
Type: changeType, | ||
}) | ||
} | ||
return events | ||
} | ||
|
||
// Helper function to test config file names cache behavior | ||
func testConfigFileNamesCacheBehavior(t *testing.T, eventPathTemplate string, expectConfigDiscovery bool, testName string) { | ||
files := map[string]any{ | ||
"/project/src/index.ts": `console.log("test");`, // No tsconfig initially | ||
} | ||
session, utils := projecttestutil.Setup(files) | ||
|
||
// Open file without tsconfig - should create inferred project | ||
session.DidOpenFile(context.Background(), "file:///project/src/index.ts", 1, files["/project/src/index.ts"].(string), lsproto.LanguageKindTypeScript) | ||
|
||
snapshot, release := session.Snapshot() | ||
defer release() | ||
assert.Assert(t, snapshot.ProjectCollection.InferredProject() != nil, "Should have inferred project") | ||
assert.Equal(t, snapshot.GetDefaultProject("file:///project/src/index.ts").Kind, project.KindInferred) | ||
|
||
// Create a tsconfig that would affect this file (simulating a missed creation event) | ||
err := utils.FS().WriteFile("/project/tsconfig.json", `{ | ||
"compilerOptions": { | ||
"strict": true | ||
}, | ||
"include": ["src/**/*"] | ||
}`, false) | ||
assert.NilError(t, err) | ||
|
||
// Create excessive changes to trigger bulk invalidation | ||
fileEvents := generateFileEvents(1001, eventPathTemplate, lsproto.FileChangeTypeCreated) | ||
|
||
// Process the changes | ||
session.DidChangeWatchedFiles(context.Background(), fileEvents) | ||
|
||
// Get language service to trigger config discovery | ||
_, err = session.GetLanguageService(context.Background(), "file:///project/src/index.ts") | ||
assert.NilError(t, err) | ||
|
||
snapshot, release = session.Snapshot() | ||
defer release() | ||
newProject := snapshot.GetDefaultProject("file:///project/src/index.ts") | ||
|
||
// Check expected behavior | ||
if expectConfigDiscovery { | ||
// Should now use configured project instead of inferred | ||
assert.Equal(t, newProject.Kind, project.KindConfigured, "Should now use configured project after cache invalidation") | ||
assert.Equal(t, newProject.Name(), "/project/tsconfig.json", "Should use the newly discovered tsconfig") | ||
} else { | ||
// Should still use inferred project (config file names cache not cleared) | ||
assert.Assert(t, newProject == snapshot.ProjectCollection.InferredProject(), "Should still use inferred project after node_modules-only changes") | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.