Skip to content

doc: update fs.watchFile doc - #40134

Closed
clement-nardi wants to merge 1 commit into
nodejs:masterfrom
clement-nardi:patch-1
Closed

doc: update fs.watchFile doc#40134
clement-nardi wants to merge 1 commit into
nodejs:masterfrom
clement-nardi:patch-1

Conversation

@clement-nardi

Copy link
Copy Markdown
Contributor

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

@nodejs-github-botnodejs-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

aduh95 commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

I mean, comparing Date objects does work in JavaScript:

newDate>=newDate;// truenewDate<=newDate;// truenewDate<newDate;// falsenewDate>newDate;// falsenewDate('1970-01-01')<newDate('2038-01-19');// truenewDate('1970-01-01')>newDate('2038-01-19');// false// Equality on the other hand doesn't worknewDate==newDate// falsenewDate('1970-01-01')==newDate('1970-01-01')// false

@clement-nardi

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
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 :)

Comment threaddoc/api/fs.md Outdated
The current wording suggests to compare Date objects, which won't work.
@targostargos added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2021
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2021
@github-actions

Copy link
Copy Markdown
Contributor

Landed in a2ed30b...2b02d2f

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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.fsIssues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@clement-nardi@aduh95@addaleax@jasnell@targos@nodejs-github-bot