-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Directories dont check modified time when sending "change" event #57938
Conversation
Directories generate change event in multiple scenarios and should not filter events based on modified time Fixes #57792
98bcd9d
to
88c5588
Compare
There was a problem hiding this comment.
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?
@DanielRosenwasser @RyanCavanaugh do we want this for 5.4 ? If we need to port this, there will be test breaks unless we take #57936 |
The cherry pick task will fix up the baselines, so that all would be handled (but we can still manually fix it) |
Just to check - is this going to be backported to 5.4 or only 5.5? |
I think it's worth considering. @typescript-bot cherry-pick this to release-5.4 |
Hey, @jakebailey! I've created #57958 for you. |
…e-5.4 (#57958) Co-authored-by: Sheetal Nandi <[email protected]> Co-authored-by: Jake Bailey <[email protected]>
Apologies for the direct mention @jakebailey, but is a new 5.4 version still planned to go out at some point? |
This was in 5.4.4 which came out last week. |
When checking whether to send event invoked with
fs.watch
we shouldnt do that for directory. Directory can sendchange
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