Fix file dedupe fourslash flake#3258
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an intermittent fourslash flake by forcing full rebuilds in cases that invalidate incremental reuse and by tightening cycle/redirect detection during program updates.
Changes:
- Force a full rebuild when
package.jsonchanges (to avoid incorrect incremental dedupe/reuse). - In
UpdateProgram, rebuild when the changed file is a redirect target as well as a redirect file.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/project/projectcollectionbuilder.go | Treats package.json edits as rebuild-triggering to prevent incorrect single-file cloning. |
| internal/compiler/program.go | Expands redirect/cycle detection to include redirectTargetsMap to avoid unsafe incremental updates. |
| // package.json changes can affect module resolution and package | ||
| // identity (e.g. dedup decisions), so they must always trigger | ||
| // a full rebuild rather than a single-file clone. | ||
| if tspath.GetBaseFileName(string(path)) == "package.json" { | ||
| dirtyFilePath = "" | ||
| break |
There was a problem hiding this comment.
The package.json check is case-sensitive, which can miss the file on case-insensitive filesystems if it appears with different casing (e.g. Package.json). Consider using a case-insensitive comparison (e.g. strings.EqualFold on the base name) or normalizing the base filename via existing path-canonicalization utilities before comparing.
There was a problem hiding this comment.
This is not quite right; if we're on a case-insensitive file system, that's when path is guaranteed to be lowercase because it's a normalized tspath.Path. Only on a case-sensitive file system would path preserve the original casing, and I think a person on a case-sensitive file system would be having a bad time with a non-standard casing of package.json anyway.
After probably 100k runs, I pinned this one down to two things:
UpdateProgramwas not fully checking if a file was a part of a cycle. Has to check both maps (Strada did too).package.jsonfile is modified, we can't reuse things anymore. It's dubious, but fourslash does actually edit JSON files, and we see them. Maybe this isn't real, but it does matter for the test at least.