gulpjs/vinyl

Document `file.stat`

demurgos opened this issue · 10 comments

demurgos commented

Hi,
file.isDirectory() mentions the file.stat property.

The property file.stat is not documented. Could you clarify this ?

demurgos commented

I saw this, but it is listed as an option, not a property.

It does not state if it is then directly accessible without breaking anything.
If it is editable, which values are accepted ? The same as when passed as an option ?

phated commented

It's only an option, not a property. It is assigned just like any other property. It is currently documented properly. If you have improvements, submit a PR for review.

demurgos commented

Well my problem it that it is the only only parameter that does break the symmetry between the options and properties: maybe there was a special reason for this ?

Since you confirm that stat is a valid property, I'll submit a PR adding it to the properties list.

phated commented

@demurgos stat is not a valid property (insomuch as a custom property tacked on). The documented properties are documented because they are getters/setters and stat is not.

It may become a getter/setter property in the future (once #105 is tackled) but is not currently and should not be documented as such.

demurgos commented

I understand that it does not have any special getter or setter, but I think that the documentation should mention it anyway.
As a user I do not need to know if it is backed by a getter or if it is a simple value: I want to know if I can do var foo = file.stat or not.

Since it is a "normal" value (not a getter/setter), I'd just describe what it means (as in options) and once you implement #105, update the documentation to specify what special behaviour was added. For the moment, I'd simply write:

#### `file.stat`

The result of an `fs.stat` call. This is how the file is marked as a directory
or symbolic link. See [isDirectory()][is-directory],
[isSymbolic()][is-symbolic] and [fs.Stats][fs-stats] for more information.

Type: [`fs.Stats`][fs-stats]

The reason why I care about that is that I contribute to the definitions of vinyl: what is available does matter. (And even if it's written that the options are copied, is not that obvious if you just want to jump to the "properties" part of the documentation)

phated commented

That's incorrect though. The property doesn't exist if you don't pass it in the options. It's more of an internal property once passed as options. That's why I don't think it belongs as documented as a property. Bad things can happen if a user changes things incorrectly.

demurgos commented

The property doesn't exist if you don't pass it in the options.

I would argue that it exists more than some other properties that made it to the documentation:

These two lines ensure that the key is there, and it is not a mere undefined but a null: it exists but does not have any value.

If I run a small REPL session, I get this:

$ node
> var Vinyl = require("vinyl");
var Vinyl = require("vinyl");
undefined
> var file = new Vinyl();
undefined
> Object.keys(file);
[ 'history', 'cwd', 'base', 'stat', '_contents', '_isVinyl' ]
> file.contents
null
> file.stat
null
> file.notAProperty
undefined

Both contents and stat are null, and what's worse: if I iterate on the keys I get stat but not contents.

If the null value still bothers you, what do you think about this formulation ?

file.stat

The result of an fs.stat call. This is how the file is marked as a directory
or symbolic link. See [isDirectory()][is-directory],
[isSymbolic()][is-symbolic] and [fs.Stats][fs-stats] for more information.

Note that its value can be null if the stat option was never defined.

Type: [fs.Stats][fs-stats] | null

.

Bad things can happen if a user changes things incorrectly.

I strongly agree on this, but even if you currently do not enforce any checks with a setter, it's better to say that "this property is for stats that are instances of fs.Stat" than to not mention it.

phated commented

I think the documentation you are proposing is incorrect. By stating "the result of an fs.stat call", it is making it seem as though we make an fs.stat call upon the path you pass in. However, I think the header "Instance properties" is also incorrect in the current docs and it should be "Instance property accessors".

Stating that you iterate over a property doesn't make for a convincing argument, as you also iterate over our _isVinyl property which is also used internally.

demurgos commented

Stating that you iterate over a property doesn't make for a convincing argument, as you also iterate over our _isVinyl property which is also used internally.

Ok, iterating over the keys does not make much sense because of the internal properties. Here is another snippet exploiting the information from Object.keys:

var Vinyl = require("vinyl");
var file = new Vinyl();

if ("stat" in file) { // This will always be true
  console.log("Stat:", file.stat);
}

Even if it is used directly here, you can easily imagine it in a function in an other file that inspects it's arguments.

I think the documentation you are proposing is incorrect. By stating "the result of an fs.stat call", it is making it seem as though we make an fs.stat call upon the path you pass in.

You are right, the above formulation was misleading. Here is an other try:

file.stat

An [fs.Stats][fs-stats] instance describing the Vinyl object. This is used to
mark the Vinyl object as a directory or a symbolic link.
See [isDirectory()][is-directory], [isSymbolic()][is-symbolic] and
[fs.Stats][fs-stats] for more information.

Note: it is null by default, you have to either pass it in the stat
option of the constructor or set it manually.

Type: [fs.Stats][fs-stats] | null

However, I think the header "Instance properties" is also incorrect in the current docs and it should be "Instance property accessors".

I think that the current wording is expected and correct according to the spec:

4.3.30 property

part of an object that associates a key (either a String value or a Symbol value) and a value
Note

Note: Depending upon the form of the property the value may be represented either directly as a data value (a primitive value, an object, or a function object) or indirectly by a pair of accessor functions.

The note clarifies that a property can be either direct or indirect, but it is still a property.