Strongly type file paths - #64159
Open
Jake Bailey (jakebailey) wants to merge 19 commits into
Open
Conversation
typescript-automation
Bot
added
Author: Team
For Milestone Bug
Copilot started reviewing on behalf of
Jake Bailey (jakebailey)
September 3, 2026 22:35
View session
Comment on lines
+426
to
+427
| checkedAbsolutePath := checkedName.WithoutRoot() | ||
| inputAbsolutePath := task.normalizedFilePath.WithoutRoot() |
Member
Author
There was a problem hiding this comment.
It's hard to pinpoint every case where we stop normalizing, but here's an example; we already know that these paths are rooted, normalized, etc, so we skip all of this, no longer need a current dir.
| type CompilerHost interface { | ||
| FS() vfs.FS | ||
| DefaultLibraryPath() string | ||
| GetCurrentDirectory() string |
Member
Author
There was a problem hiding this comment.
It's pretty amazing that we don't need this at all.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Path normalization, relative auto-import rebasing, and case-insensitive watcher invalidation have unresolved correctness defects.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces strongly typed rooted paths and canonical path keys throughout the compiler, language server, VFS, and unstable TypeScript API.
Changes:
- Adds typed-path primitives,
CaseSensitivity, conversion helpers, and lint enforcement. - Propagates typed paths through resolution, emit, watching, LSP, and API boundaries.
- Adds regression tests and updates generated baselines.
File summaries
| File group | Description |
|---|---|
tsc/internal/tspath/* |
Defines typed paths and path operations. |
tsc/internal/{compiler,module,checker,ast,binder,parser,printer,sourcemap,transformers}/* |
Migrates compiler internals. |
tsc/internal/{ls,lsp,project,contentmapper}/* |
Migrates language-service boundaries. |
tsc/internal/{vfs,execute,transpile,bundled}/* |
Migrates filesystem and execution paths. |
tsc/internal/{testutil,testrunner,fourslash,format}/* |
Updates test infrastructure and cases. |
tsc/testdata/tests/cases/compiler/* |
Adds path regression scenarios. |
tsc/testdata/baselines/reference/* |
Updates expected compiler and LSP output. |
packages/typescript/src/* |
Exposes typed paths in the unstable API. |
packages/typescript/test/* |
Updates JavaScript API tests and benchmarks. |
tools/customlint/* |
Enforces typed-path invariants. |
tools/{gen-proto,scripts/tsc}/*, Herebyfile.mjs |
Updates generators and generated enums. |
tsc/cmd/tsc/* |
Converts process-level path boundaries. |
Review details
- Files reviewed: 169/449 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Jake Bailey (jakebailey)
force-pushed
the
typed-paths
branch
from
September 4, 2026 21:24
e4c4ac8 to
cffbf0c
Compare
| * Handle format: "index.kind.path" where path may contain dots. | ||
| */ | ||
| export function parseNodeHandle(handle: string): ParsedNodeHandle { | ||
| export function parseNodeHandleFromCompiler(handle: string): ParsedNodeHandle { |
Member
Author
There was a problem hiding this comment.
Need to double check what the heck is going on here
Jake Bailey (jakebailey)
force-pushed
the
typed-paths
branch
2 times, most recently
from
September 4, 2026 23:43
74c07a3 to
c11f029
Compare
Copilot started reviewing on behalf of
Jake Bailey (jakebailey)
September 5, 2026 01:21
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
CommonDirectoryOfFiles treats case-equivalent drive roots as unrelated, potentially corrupting common-source and emit-path computation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 164/668 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+593
to
+608
| effectiveCaseSensitivity := c | ||
| if IsEncodedDynamicFileName(fileName.AsString()) || | ||
| IsEncodedDynamicFileName(GetPathFromPathComponents(commonPathComponents)) { | ||
| effectiveCaseSensitivity = CaseSensitive | ||
| commonPathComponents[0] = strings.TrimSuffix(commonPathComponents[0], string(DirectorySeparator)) | ||
| pathComponents[0] = strings.TrimSuffix(pathComponents[0], string(DirectorySeparator)) | ||
| } | ||
| for i := range n { | ||
| if effectiveCaseSensitivity.Canonicalize(commonPathComponents[i]) != effectiveCaseSensitivity.Canonicalize(pathComponents[i]) { | ||
| if i == 0 { | ||
| return "" | ||
| } | ||
| commonPathComponents = commonPathComponents[:i] | ||
| break | ||
| } | ||
| } |
…DA BUG) Main and the pre-port Strada behavior use canonical path strings as lookup keys, but some callers also reused those keys when constructing watcher globs or substituting a symlink target. On case-insensitive hosts that loses the spelling supplied by the filesystem or project configuration. Track the original filename alongside each canonical identity. Build watcher patterns from presentation paths, and retain the symlink spelling so child suffixes are taken from the original filename while lookup still uses canonical keys. This keeps identity comparisons canonical without leaking canonical casing into user-visible paths or filesystem watch registrations. Category: Typed-path-discovered; inherited Strada bug
…HS, STRADA BUG) Add focused coverage for the package realpath cache used during auto-import discovery. Exercise unscoped packages, scoped packages, and direct files under node_modules so file paths cannot silently become package-directory cache entries. The pre-fix expectations record the existing missing-separator result, which keeps this test commit independently green. The following fix updates the expectations to the path-preserving behavior. Category: Typed-path-discovered; inherited Strada bug
…S, STRADA BUG) Main and the pre-port Strada implementation use the same string path shape for both files and package directories while populating the auto-import realpath cache. A direct file under node_modules can therefore be interpreted as a package root, and replacing a cached prefix with string concatenation can produce paths such as node_modulesdep with no directory separator. Check whether each traversal candidate is actually a directory before caching a package realpath. Distinguish file and directory package-root parsing, and substitute cached prefixes with path-aware joining so exact roots and descendants preserve their separators. Cover bare scope directories, scoped package roots, unscoped packages, and direct files under node_modules. Category: Typed-path-discovered; inherited Strada bug
…DA BUG) Add coverage for deriving node_modules package roots from both file paths and directory paths. Include unscoped packages, scoped packages, bare scope directories, and direct files under node_modules. The test records the old directory parser's trailing-separator result so it remains independently green before the API is replaced. Category: Typed-path-discovered; inherited Strada bug
…DA BUG) Main and Strada route both files and directories through ParseNodeModuleFromPath with a Boolean path-kind argument. That makes it easy for callers to select the wrong boundary, especially for direct files under node_modules and scoped package directories. Replace the Boolean contract with explicit NodeModulePackageRootForFile and NodeModulePackageRootForDirectory helpers. Update module resolution, rename, auto-import realpaths, and project-reference source mapping to call the helper matching the path they hold. The explicit boundary prevents files from poisoning directory caches and keeps bare scopes distinct from scoped package roots. Category: Typed-path-discovered; inherited Strada bug
…A BUG) Add declaration-emit coverage for a type reached through node_modules/foo/other/index.d.ts. Exercise both a plain child directory and a child with its own package.json. Without nested metadata, the emitted specifier must retain the file path instead of treating foo/other as a package entrypoint. With nested metadata, the child package may still define that entrypoint. Category: Typed-path-discovered; inherited Strada bug
… BUG) The index-based node_modules path analysis inherited by main and Strada updates PackageRootIndex while probing nested package.json files. When a nested package.json is absent, that loses the original package boundary and incorrectly treats a child index.d.ts as the child directory's package entrypoint, shortening foo/other/index.d.ts to foo/other. Track the current candidate package root separately from the original package base. Use the original base when deciding whether a file without nested metadata is a package index, while still honoring actual nested package.json entrypoints. This keeps generated module specifiers stable for ordinary package subdirectories without disabling valid nested packages. Category: Typed-path-discovered; inherited Strada bug
… PARITY) Add a tsc incremental project-reference baseline for a nested source file with a triple-slash reference to referenced-project source. The baseline records the incorrect source dependency retained when redirect lookup roots the reference at the program working directory. Category: Typed-path-discovered; restores Strada behavior
…PARITY) Resolve triple-slash reference text from the containing source file directory before looking up project-reference redirects. This restores the output dependency intended by the original builder redirect change and removes the redundant referenced-project source from incremental build info. Category: Typed-path-discovered; restores Strada behavior
…ADA PARITY) The native API accepted project-relative file handles, but prepared auto-import snapshots by converting the unresolved handle against the session working directory. Strada and the TypeScript API instead operate on the source file selected from the project program, so the retry could prepare the wrong document and return no edit. Convert the resolved source filename back to a document URI before preparing auto imports. Apply the same rule to the completion retry added on current main, and cover both synchronous and asynchronous project-relative API calls. Category: Typed-path-discovered; restores Strada behavior
…IVE API) The JavaScript API accepts project-relative document identifiers, but its source-file cache canonicalized those identifiers against the API session's working directory. A project whose current directory differs from the session therefore fetched the same server source file under a second cache key and returned a new wrapper instead of preserving object identity. The native compiler and Strada resolve source files in their project context; the bug was confined to the newer JavaScript API cache boundary on main. Allow the API path converter to take an explicit base directory, and use the owning project's current directory for source- file and metadata cache keys. Keep the session directory as the default for project and snapshot keys. Cover both synchronous and asynchronous APIs, including metadata lookup and cache reuse across project-relative and absolute identifiers. Category: Typed-path-discovered; native API bug
…EW, STRADA BUG) Treat the file scheme and localhost authority case-insensitively when identifying local file URL volume roots. This matches URL semantics and prevents uppercase spellings from changing path normalization behavior. Category: Review-discovered; inherited Strada bug
Follow ECMA-426 when decoding source URLs. Preserve null source entries, resolve empty references with the specified sourceRoot semantics, retain duplicate source indices, and treat invalid source indices as unmapped positions rather than indexing invalid data. Omit sourceRoot from generated maps when it is not configured. The standard distinguishes an absent value from an explicit empty string, which denotes the root prefix "/"; historically TypeScript emitted an empty string while intending the absent-value behavior. Keep reverse mapping storage sparse-safe and reject malformed JSON values. Retain map-relative fallback for published TypeScript maps that used an explicit empty sourceRoot with nonempty relative sources. Category: Review-discovered; inherited Strada bug
…DA BUG) Strada and main flattened non-file URIs into synthetic path strings. That representation normalized away dot segments and repeated separators, conflated reserved-looking names with encoded names, and could lose query, fragment, authority-only, and case-sensitive identity. The TypeScript API mirror had the same behavior. Introduce a versioned, reversible dynamic URI encoding in both Go and the TypeScript API. Treat the scheme and authority as the synthetic root, keep dynamic identities case-sensitive, and translate explicitly between logical URI segments and physical resolver paths. Carry that distinction through rootDirs, package.json fields, exports, generated entrypoints, CommonJS directory lookup, and VFS glob matching. Legacy synthetic names remain literal and retain their previous decoding behavior. Cover URI round trips, exceptional and reserved segments, dynamic package paths, rootDirs transitions, dotted directories, and generated module specifiers. Category: Review-discovered; inherited Strada bug
…NLY) API project and file opens are ref-counted in snapshot state. The project collection builder cloned that state for each snapshot, but then mutated the clone incrementally while processing closes and opens. If a later project update failed, the errored snapshot retained the earlier ref-count changes. The API session commits its own open-resource bookkeeping only after a successful update. Adopting the partially updated snapshot therefore left the two layers inconsistent, allowing a later close to release another session's reference or keep a resource loaded indefinitely. Clone the API state again at the request boundary and restore the pre-request state on error. This behavior is in the current native API on main; Strada did not have this shared, ref-counted API snapshot mechanism. Add a regression test that forces an update failure after closing a project and verifies that every API reference remains unchanged. Category: Review-discovered; native-only bug
An API update filtered new project and file opens only against resources already held by the session. It did not deduplicate aliases within the same request. On a case-insensitive host, two differently cased identifiers could therefore increment the same underlying PathKey reference twice while the API session recorded only one key. Closing that session released the canonical key once and leaked the second reference, leaving the project or file loaded. Equivalent normalized spellings could produce the same mismatch whenever their presentation values remained distinct in the request set. Track canonical project and file keys while building each update request and send only the first presentation value for each identity. Add case-insensitive project and file alias tests that verify session close releases the resource. This bug is present in the shared native API snapshot implementation on main; Strada did not have this ref-counted multi-session API layer. Category: Review-discovered; native-only bug
A project session owns the initial reference to its current snapshot. Snapshot replacement releases the old session-owned reference, but Session.Close previously closed only the SnapshotHost and left the final current snapshot referenced. That retained the snapshot program, parse-cache entries, checker pools, and related project resources after the session itself had closed. Cancel and join background work, serialize closure against snapshot updates, then detach and dereference the current snapshot before closing its host. Thread the session context through automatic type acquisition and npm execution so shutdown can terminate external installs rather than waiting indefinitely. Make queue closure reject new work atomically with waiting for accepted work. LSP API sessions share the project snapshot and release their open-resource references through another update. Track their transports and connections so shutdown can stop and await all request handlers before closing each child session and finally the project session. This leak is present in the native project session on main. Strada managed project state through its server session lifecycle and did not have this reference-counted native snapshot ownership model. Add regression coverage that Close waits for queued work and that closing a project session releases both its parse-cache entry and program reference. Category: Review-discovered; native-only bug
…IVE ONLY) Content mapper manifest invalidation compared watch event paths with package manifest paths using their presentation strings. On a case-insensitive filesystem, equivalent paths with different casing therefore failed to trigger a mapper reload. Canonicalize both the changed paths and each configured mapper manifest using the watcher comparison policy before looking them up. Add a case-insensitive watch test whose symlink target and emitted event differ only in casing. This bug is present on main and is independent of typed paths. Category: Review-discovered; native-only bug
Replace ambiguous string path contracts with a typed lattice for rooted files, rooted directories, normalized relative paths, and canonical path keys. Keep canonical identity as a one-way sink while retaining presentation spelling wherever diagnostics, watches, symlinks, or protocol responses need it. Carry those invariants through compiler inputs and outputs, module resolution, project snapshots, language-service hosts, VFS operations, source maps, LSP conversion, and the JavaScript API. Separate raw compiler option wire values from finalized rooted options, and centralize explicit normalization, rooting, and case-sensitivity boundaries. This commit consolidates the exploratory migration into one reviewable rewrite after the independently portable fixes. It also adapts those fixes to the typed representation and retains the two newer main changes, including auto-import completion retries and tuple completion filtering. Category: Typed-path migration
Jake Bailey (jakebailey)
force-pushed
the
typed-paths
branch
from
September 5, 2026 04:38
c11f029 to
6f16b34
Compare
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 a wacky change I've wanted to try out for a while and finally started screwing around with with copilot.
Right now (and in Strada), we have just two kinds of paths:
string- 🤷Path- an OS dependent string used for map keys, lowercased on case insensitive systemsOur use of
stringpaths led to us slappingnormalizeSlashes,normalizePath, etc everywhere, as we often were unsure (or pessimistic) whether or not a path had its slashes normalized to/, had redundant components removed, trailing slashes removed, not relative, etc. This is extra bad because on Linux, macOS, etc, paths are basically guaranteed to meet all of the criteria, but we'd try and normalize them anyway.This PR changes this by introducing named/branded types for paths which assert properties about those paths. This is not a new concept; I believe yarn's FS package has this, and I'm sure others do.
As a hierarchy:
string- No guarantees.RootedPath- The path is absolute, has normalized slashes, no trailing/.RootedFilePath- ARootedPath, but indicates that the path is supposed to point at a file.RootedDirectoryPath- ARootedPath, but indicates that the path is supposed to point at a directory.PathKey- Same as the oldPath, but renamed for clarity.This is a big refactor that requires changing a lot of code, but leads to some pretty important properties.
Paths are converted at the boundaries, e.g. paths provided via config files, CLI, from the OS, the editor, etc. Once converted, you always know exactly what format a path is in and therefore never need to normalize again.
Paths are always rooted. The "current working directory" does not need to be plumbed around as much anymore, since most uses were simply to root paths we were unsure about.
Since paths are always rooted,
ComparePathsOptions's current dir field is no longer needed! This means comparing paths only requiresUseCaseSensitiveFileNames. This applies also to all of our oldtoPathconversions, since we only ever need to canonicalize rooted paths. So, I created a newCaseSensitivityenum, and then all of the plumbing forComparePathsOptions, its working dir, etc, also get to go away.The impact of this is measurable; I instrumented main vs my branch to count how many of the normalizing operations go away and it's a lot:
Old compiler fixture
maintyped-pathsCombinePathsNormalizePathNormalizeSlashesVS Code
srcmaintyped-pathsCombinePathsNormalizePathNormalizeSlashesThat's millions of normalizations that no longer need to happen. In terms of runtime, it's not a lot of savings, even on Windows, but I did also measure about a 7% speedup in program load of the old compiler, which is nice.
Additionally, the strong typing here caught 3 different bugs that have been around in main for a while, places where we had mixed up paths, rooted them relative to the wrong directory, etc. Those are denoted in my (awful) git history as being things to port to main, which I may still do.
In addition to just the types themselves, a new lint rule bans manually hacking on the paths; all operations should go through methods on the paths themselves. No concat, splitting, conversions, yourself.
The downside here is just churning the API and introducing these concepts to downstream API users. But the strong typing itself I think is worth it, and doing a lot less work is a bonus too. We probably won't have a change to do something like this for a while.
I'm also going to say that this fixes #44174 just since this eliminates nearly all normalization; we might still do a quick check at the boundaries, but other than that, we never normalize gain.