Skip to content
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

doc: update fs.watchFile doc #40134

Closed
wants to merge 1 commit into from

Conversation

clement-nardi
Copy link
Contributor

The current wording suggests to compare Date objects, which won't work.

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Sep 17, 2021
@aduh95
Copy link
Contributor

aduh95 commented Sep 17, 2021

I mean, comparing Date objects does work in JavaScript:

new Date >= new Date; // true
new Date <= new Date; // true
new Date < new Date; // false
new Date > new Date; // false
new Date('1970-01-01') < new Date('2038-01-19'); // true
new Date('1970-01-01') > new Date('2038-01-19'); // false

// Equality on the other hand doesn't work
new Date == new Date // false
new Date('1970-01-01') == new Date('1970-01-01') // false

@clement-nardi
Copy link
Contributor Author

Well, here the idea is to know whether a file has been modified by comparing modified times.
I think most people would use the inequality operator for this:

if (prev.mtime !== curr.mtime) {
    console.log("The file has been modified!")
}

which doesn't work.

Actually that's the root cause of a bug I just fixed.

@addaleax
Copy link
Member

Keep in mind that "compare X and Y" does not mean "apply a JS comparison operator between X and Y"; for example, X.getTime() === Y.getTime() is a way to compare X and Y.

In any case, this can be sidestepped by just referring to curr.mtimeMs and prev.mtimeMs instead of curr.mtime and prev.mtime :)

doc/api/fs.md Outdated Show resolved Hide resolved
The current wording suggests to compare Date objects, which won't work.
@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2021
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2021
@github-actions
Copy link
Contributor

Landed in a2ed30b...2b02d2f

@github-actions github-actions bot closed this Sep 26, 2021
nodejs-github-bot pushed a commit that referenced this pull request Sep 26, 2021
The current wording suggests to compare Date objects, which won't work.

PR-URL: #40134
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this pull request Oct 4, 2021
The current wording suggests to compare Date objects, which won't work.

PR-URL: #40134
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants