Skip to content

Directories dont check modified time when sending "change" event - #57938

Merged
Sheetal Nandi (sheetalkamat) merged 2 commits into
mainfrom
useFsEventsOnParentDirectory
Mar 25, 2024
Merged

Sheetal Nandi (sheetalkamat) merged 2 commits into
mainfrom
useFsEventsOnParentDirectory

Conversation

@sheetalkamat

@sheetalkamat Sheetal Nandi (sheetalkamat) commented Mar 25, 2024

Copy link
Copy Markdown
Member

When checking whether to send event invoked with fs.watch we shouldnt do that for directory. Directory can send change event for multiple reasons and its not right to filter that out.

a7a308d Adds the failing test. It modifies out vfs sending incorrect timestamps, incorrect events etc.
88c5588 actual fix

Fixes #57792

@typescript-bot TypeScript Bot (typescript-bot) added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Mar 25, 2024
@sheetalkamat Sheetal Nandi (sheetalkamat) changed the title Use fs events on parent directory Directories dont check modified time when sending "change" event Mar 25, 2024
Base automatically changed from watchTestsTimeStamp to main March 25, 2024 21:11
Directories generate change event in multiple scenarios and should not filter events based on modified time
Fixes #57792

@jakebailey Jake Bailey (jakebailey) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a backport to 5.4?

@sheetalkamat
Sheetal Nandi (sheetalkamat) deleted the useFsEventsOnParentDirectory branch March 25, 2024 23:19
@sheetalkamat

Copy link
Copy Markdown
Member Author

Daniel Rosenwasser (@DanielRosenwasser) Ryan Cavanaugh (@RyanCavanaugh) do we want this for 5.4 ? If we need to port this, there will be test breaks unless we take #57936

@jakebailey

Copy link
Copy Markdown
Member

The cherry pick task will fix up the baselines, so that all would be handled (but we can still manually fix it)

@NMinhNguyen

Copy link
Copy Markdown
Contributor

Just to check - is this going to be backported to 5.4 or only 5.5?

@jakebailey

Copy link
Copy Markdown
Member

I think it's worth considering.

TypeScript Bot (@typescript-bot) cherry-pick this to release-5.4

@typescript-bot

TypeScript Bot (typescript-bot) commented Mar 27, 2024

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
cherry-pick this to release-5.4 ✅ Started ✅ Results

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey, Jake Bailey (@jakebailey)! I've created #57958 for you.

Daniel Rosenwasser (DanielRosenwasser) pushed a commit that referenced this pull request Mar 28, 2024
…e-5.4 (#57958)

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
@NMinhNguyen

Copy link
Copy Markdown
Contributor

Apologies for the direct mention Jake Bailey (@jakebailey), but is a new 5.4 version still planned to go out at some point?

@jakebailey

Copy link
Copy Markdown
Member

This was in 5.4.4 which came out last week.

@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsc --watch via directory-level fsevents not working on macOS 14.4

5 participants