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

fs: expose Stats times as Numbers #13173

Merged
merged 1 commit into from
Jun 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,14 @@ argument to `fs.createReadStream()`. If `path` is passed as a string, then
## Class: fs.Stats
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/13173
description: Added times as numbers.
-->

Objects returned from [`fs.stat()`][], [`fs.lstat()`][] and [`fs.fstat()`][] and their
synchronous counterparts are of this type.
Objects returned from [`fs.stat()`][], [`fs.lstat()`][] and [`fs.fstat()`][] and
their synchronous counterparts are of this type.

- `stats.isFile()`
- `stats.isDirectory()`
Expand All @@ -329,20 +333,23 @@ Stats {
size: 527,
blksize: 4096,
blocks: 8,
atimeMs: 1318289051000.1,
mtimeMs: 1318289051000.1,
ctimeMs: 1318289051000.1,
birthtimeMs: 1318289051000.1,
atime: Mon, 10 Oct 2011 23:24:11 GMT,
mtime: Mon, 10 Oct 2011 23:24:11 GMT,
ctime: Mon, 10 Oct 2011 23:24:11 GMT,
birthtime: Mon, 10 Oct 2011 23:24:11 GMT }
```

Please note that `atime`, `mtime`, `birthtime`, and `ctime` are
instances of [`Date`][MDN-Date] object and appropriate methods should be used
to compare the values of these objects. For most general uses
[`getTime()`][MDN-Date-getTime] will return the number of milliseconds elapsed
since _1 January 1970 00:00:00 UTC_ and this integer should be sufficient for
any comparison, however there are additional methods which can be used for
displaying fuzzy information. More details can be found in the
[MDN JavaScript Reference][MDN-Date] page.
*Note*: `atimeMs`, `mtimeMs`, `ctimeMs`, `birthtimeMs` are [numbers][MDN-Number]
that hold the corresponding times in milliseconds. Their precision is platform
specific. `atime`, `mtime`, `ctime`, and `birthtime` are [`Date`][MDN-Date]
object alternate representations of the various times. The `Date` and number
Copy link
Contributor

@mscdex mscdex May 26, 2017

Choose a reason for hiding this comment

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

I'm thinking maybe we should just drop the whole bit about connected-ness now, since technically the Date objects are connected at least until first use (since they draw from the number-based properties)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But we do call them alternate representation which could imply the same underlying state...
We could go with a more radical approach, not cache the Dates and remove the setters. Then say that the Date fields are ephemeral and mutating them has no effect.
I don't see a really obvious use case for (1) accessing a Date several times (2) mutating the Date for downstream processing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts @nodejs/collaborators?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With current changeset this is a moot point for now. We should move connection discussion to #12818

values are not connected. Assigning a new number value, or mutating the `Date`
value, will not be reflected in the corresponding alternate representation.
Copy link
Member

@addaleax addaleax May 26, 2017

Choose a reason for hiding this comment

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

This sounds like you could actually change the time of a Date object, but I am pretty sure that’s not possible?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are date.setTime(), date.setHours(), etc.



### Stat Time Values

Expand Down Expand Up @@ -2841,6 +2848,7 @@ The following constants are meant for use with the [`fs.Stats`][] object's
[FS Constants]: #fs_fs_constants_1
[MDN-Date-getTime]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Date/getTime
[MDN-Date]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Date
[MDN-Number]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type
[MSDN-Rel-Path]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247.aspx#fully_qualified_vs._relative_paths
[Readable Stream]: stream.html#stream_class_stream_readable
[Writable Stream]: stream.html#stream_class_stream_writable
Expand Down
4 changes: 4 additions & 0 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ function Stats(
this.ino = ino;
this.size = size;
this.blocks = blocks;
this.atimeMs = atim_msec;
this.mtimeMs = mtim_msec;
this.ctimeMs = ctim_msec;
this.birthtimeMs = birthtim_msec;
this.atime = new Date(atim_msec + 0.5);
this.mtime = new Date(mtim_msec + 0.5);
this.ctime = new Date(ctim_msec + 0.5);
Expand Down
57 changes: 37 additions & 20 deletions test/parallel/test-fs-stat.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,38 +65,55 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) {
assert.fail(err);
}
if (stats) {
console.dir(stats);
assert.ok(stats.mtime instanceof Date);
}
fs.close(fd, assert.ifError);
}));

console.log(`stating: ${__filename}`);
fs.stat(__filename, common.mustCall(function(err, s) {
assert.ifError(err);

console.dir(s);

console.log(`isDirectory: ${JSON.stringify(s.isDirectory())}`);
assert.strictEqual(false, s.isDirectory());

console.log(`isFile: ${JSON.stringify(s.isFile())}`);
assert.strictEqual(true, s.isFile());

console.log(`isSocket: ${JSON.stringify(s.isSocket())}`);
assert.strictEqual(false, s.isSocket());

console.log(`isBlockDevice: ${JSON.stringify(s.isBlockDevice())}`);
assert.strictEqual(false, s.isBlockDevice());

console.log(`isCharacterDevice: ${JSON.stringify(s.isCharacterDevice())}`);
assert.strictEqual(false, s.isCharacterDevice());

console.log(`isFIFO: ${JSON.stringify(s.isFIFO())}`);
assert.strictEqual(false, s.isFIFO());

console.log(`isSymbolicLink: ${JSON.stringify(s.isSymbolicLink())}`);
assert.strictEqual(false, s.isSymbolicLink());

assert.ok(s.mtime instanceof Date);
const keys = [
'dev', 'mode', 'nlink', 'uid',
'gid', 'rdev', 'ino', 'size',
'atime', 'mtime', 'ctime', 'birthtime',
'atimeMs', 'mtimeMs', 'ctimeMs', 'birthtimeMs'
];
if (!common.isWindows) {
keys.push('blocks', 'blksize');
}
const numberFields = [
'dev', 'mode', 'nlink', 'uid', 'gid', 'rdev', 'ino', 'size',
'atimeMs', 'mtimeMs', 'ctimeMs', 'birthtimeMs'
];
const dateFields = ['atime', 'mtime', 'ctime', 'birthtime'];
keys.forEach(function(k) {
assert.ok(k in s, `${k} should be in Stats`);
assert.notStrictEqual(s[k], undefined, `${k} should not be undefined`);
assert.notStrictEqual(s[k], null, `${k} should not be null`);
});
numberFields.forEach((k) => {
assert.strictEqual(typeof s[k], 'number', `${k} should be a number`);
});
dateFields.forEach((k) => {
assert.ok(s[k] instanceof Date, `${k} should be a Date`);
});
const jsonString = JSON.stringify(s);
const parsed = JSON.parse(jsonString);
keys.forEach(function(k) {
assert.notStrictEqual(parsed[k], undefined, `${k} should not be undefined`);
assert.notStrictEqual(parsed[k], null, `${k} should not be null`);
});
numberFields.forEach((k) => {
assert.strictEqual(typeof parsed[k], 'number', `${k} should be a number`);
});
dateFields.forEach((k) => {
assert.strictEqual(typeof parsed[k], 'string', `${k} should be a string`);
});
}));