Skip to content

Conversation

@DanieleAurilio
Copy link
Contributor

The condition was not working correctly because it compared two Date objects directly, which always results in false even if they represent the same timestamp. The fix uses .getTime() to compare numeric values instead.

Ref: #29421

@bartlomieju
Copy link
Member

Thanks for the PR! Do you think there's any chance we could test it somehow? @kt3k @littledivy maybe there's a Node compat test that we could enable now?

@kt3k
Copy link
Member

kt3k commented Jun 11, 2025

Added assertions to exercise this fix.

@kt3k kt3k changed the title fix(node/fs): fix node fs watchFile trigger fix(ext/node): fix fs.watchFile trigger Jun 11, 2025
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k merged commit e274479 into denoland:main Jun 11, 2025
18 checks passed
bartlomieju added a commit that referenced this pull request Jun 11, 2025
This test was added in #29659 and
an attempt to make it more stable was done in
#29699.

Unfortunately it's still very flaky. Ignoring for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants