Deno: Remove FileInfo.path

[bartlomieju]

Discussion from gitter chat.

Calls to stat and lstat return FileInfo with null values for name and path.

const fileInfo = deno.stat("./log.txt");
fileInfo.name === null;
fileName.path === null;

Call to readDir returns FileInfo[] with name and path filled.

const dirInfo = deno.readDir(".");
const fileInfo = dirInfo[0];
fileInfo.name === "<filename>";
fileName.path === "<file path>";

stat does not return values for those two fields, which might lead to confusion down the road. It'd be better not to do additional syscall during stat.

CC @ry

[ry]

Also why do we have both name and path in FileInfo?
https://github.com/denoland/deno/blob/e782ba1a6fd8342cddd961da9274dbd0244685d1/js/file_info.ts#L30-L34

[bartlomieju]

Isn't it like this?

/var/log/postgres/log.txt
name === "log.txt"
path === "/var/log/postgres/log.txt"

[ry]

That seems like something people can compute themselves. We shouldn't be doing more than the minimal amount of work.

Note the structure of https://golang.org/pkg/os/#FileInfo

[bartlomieju]

Golang basically removes leading directory and trailing slashes from name given to stat.

[kitsonk]

As @GrosSacASac pointed out in Gitter, these could be memoized getters, so the first time they are accessed they are calced/retrieved and stored. Of course it maybe just better to have them as functions and people then understand there is some sort of _cost_ to retrieving them. It is certainly doable with getters, but users don't consciously realise there is a cost.

[hayd]

I think we can stub both these in on both stat and lstat calls, since we're passed the filename (.path) and can take the .name from that too.

https://github.com/denoland/deno/commit/b723ea388a055ad3f2d3fb7aa55cad639329992c

[arcatdmz]

I personally prefer retaining the path property since functions like this can be simpler without the first filePath argument.

Meanwhile, given that #1774 was closed without getting merged, it seems like the conclusion is (unfortunately) already made that we're removing these properties from FileInfo. If so, this issue could also be closed.

[hayd]

Unfortunately I think this logic needs to be moved to something in std. I don't really understand @ry's decision as I think it's useful, trivial and without perf impact, but looks like this convenience needs to be wrapped around every fs function in std (with a FileInfo type defined there which includes both path/name fields). 🙄

[ry]

FileInfo doesn't need both the base name and the path. It's redundant.

If you want to have Stat copy the base name into the FileInfo, I'd be fine with that. But first, FileInfo.path needs to be removed.

[arcatdmz]

I thought stat and lstat resolve the provided path inside their implementation anyway, and returning the resolution result (full path) as the path property does not have any perf impact.

If things are not like this (it seems like so), I'm totally convinced that FileInfo.path can be removed.

[ry]

I am removing FileInfo.path
https://github.com/denoland/deno_std/pull/398
https://github.com/denoland/deno/pull/2313