Remove FS().WalkDir - #64277
Merged
Andrew Branch (andrewbranch) merged 3 commits intoSep 15, 2026
Merged
Remove FS().WalkDir#64277
Conversation
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Andrew Branch (andrewbranch)
requested a review
from Jake Bailey (jakebailey)
September 15, 2026 14:01
Copilot started reviewing on behalf of
Andrew Branch (andrewbranch)
September 15, 2026 14:02
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new helper mishandles top-level skip sentinels and can report the wrong name for a symlink root.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Removes WalkDir from the VFS interface and replaces required traversal with a composable helper based on cached VFS primitives.
Changes:
- Removes
WalkDirimplementations, mocks, and legacy tests. - Adds
vfs.WalkDirwith symlink-aware traversal tests. - Migrates LSP, fourslash, and configuration callers.
File summaries
| File | Description |
|---|---|
tsc/internal/vfs/wrapvfs/wrapvfs.go |
Removes WalkDir wrapping. |
tsc/internal/vfs/walkdir.go |
Adds traversal helper. |
tsc/internal/vfs/walkdir_test.go |
Tests traversal behavior. |
tsc/internal/vfs/vfstest/vfstest_test.go |
Updates stress operations. |
tsc/internal/vfs/vfsmock/wrapper.go |
Removes mock delegation. |
tsc/internal/vfs/vfsmock/mock_generated.go |
Removes generated mock API. |
tsc/internal/vfs/vfs.go |
Shrinks the FS interface. |
tsc/internal/vfs/trackingvfs/trackingvfs.go |
Removes tracking wrapper. |
tsc/internal/vfs/osvfs/os.go |
Removes OS traversal implementation. |
tsc/internal/vfs/iovfs/iofs.go |
Removes IOFS traversal. |
tsc/internal/vfs/iovfs/iofs_test.go |
Removes obsolete tests. |
tsc/internal/vfs/internal/internal.go |
Removes common traversal. |
tsc/internal/vfs/cachedvfs/cachedvfs.go |
Removes uncached delegation. |
tsc/internal/vfs/cachedvfs/cachedvfs_test.go |
Removes delegation tests. |
tsc/internal/tsoptions/tsconfigparsing_test.go |
Migrates filesystem printing. |
tsc/internal/project/snapshotfs.go |
Removes sourceFS traversal. |
tsc/internal/lsp/lspwatcher/lspwatcher.go |
Migrates synthetic-create traversal. |
tsc/internal/fourslash/fourslash.go |
Uses accessible-file collection. |
tsc/internal/fourslash/baselineutil.go |
Adds shared file collection. |
tsc/internal/compiler/projectreferencedtsfakinghost.go |
Removes unsupported method. |
tsc/internal/bundled/generate.go |
Stops generating directory entries. |
tsc/internal/bundled/embed.go |
Removes embedded traversal. |
tsc/internal/bundled/embed_generated.go |
Updates generated output. |
tsc/internal/bundled/bundled_test.go |
Tests direct directory listing. |
tsc/internal/api/requestfilesystem/requestfilesystem.go |
Removes bespoke traversal. |
tsc/internal/api/requestfilesystem/requestfilesystem_test.go |
Updates request-FS tests. |
tsc/internal/api/requestfilesystem/pathtree_test.go |
Replaces traversal assertions. |
tsc/internal/api/callbackfs.go |
Removes base delegation. |
Review details
Files not reviewed (3)
- tsc/internal/bundled/embed_generated.go: Generated file
- tsc/internal/bundled/generate.go: Generated file
- tsc/internal/vfs/vfsmock/mock_generated.go: Generated file
Suppressed comments (1)
tsc/internal/vfs/walkdir.go:140
fs.WalkDirtreatsSkipDirreturned for a root file (or root symlink) as a successful stop, but this only consumesSkipAll;WalkDirtherefore unexpectedly returnsfs.SkipDirto the caller. Consume both sentinels at the top level, matching the nested-file handling andfs.WalkDirsemantics.
if err := visit(root, rootEntry, rootRealpath); errors.Is(err, fs.SkipAll) {
- Files reviewed: 25/28 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Copilot started reviewing on behalf of
Andrew Branch (andrewbranch)
September 15, 2026 15:24
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The interface removal is consistently applied, callers are migrated, and the replacement traversal behavior has focused coverage.
Review details
Files not reviewed (3)
- tsc/internal/bundled/embed_generated.go: Generated file
- tsc/internal/bundled/generate.go: Generated file
- tsc/internal/vfs/vfsmock/mock_generated.go: Generated file
- Files reviewed: 25/28 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Jake Bailey (jakebailey)
approved these changes
Sep 15, 2026
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
Member
There was a problem hiding this comment.
I think this makes sense.
Member
Author
|
I'll wait for #64269 to merge. |
Member
Author
|
Never mind, #64285 could use the helper. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is something I wanted to do way back when I was implementing the LSP infrastructure because I noticed that
WalkDirhad no production callers, and wasn't possible to cache incachedvfs, which would make it prone to violating snapshot guarantees if it was implemented. Similar friction came up again in #64269, where it's just difficult to implement WalkDir correctly in a highly composed filesystem, and then I remembered that we don't have to.Unfortunately, I later added one production caller (🤦🏻♂️), but it’s easy to remove. In the second commit, I tried making a top-level
vfs.WalkDirhelper function that implements WalkDir-like behavior in terms ofGetAccessibleEntries,Realpath, andStat. It does make the previous callers (one production and a few test) simpler, but I'm not sure if it's worth it. Without the second commit, this PR is almost all deletion (-638 +96).