whatwg / fs

File System Standard
https://fs.spec.whatwg.org/
Other
237 stars 20 forks source link

Give FileSystemHandles an associated path #96

Closed a-sully closed 1 year ago

a-sully commented 1 year ago

Fixes #59

A much more thorough (than #30) attempt at expressing that FileSystemHandles map to file paths. At a high level, it proposes that:

See #59 for the implications of this in practice.

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

a-sully commented 1 year ago

I expect the build to fail for now, since [=FileSystemHandle/entry=] has been un-defined. I plan to update all the uses once we decide on how a FileSystemHandle should relate to an entry

a-sully commented 1 year ago

@annevk do you have any idea why the build is failing? Does it not like "getting the entry"?

annevk commented 1 year ago

I suspect it's failing because of "the the". Search for "typos" in https://resources.whatwg.org/build/deploy.sh.

a-sully commented 1 year ago

I suspect it's failing because of "the the". Search for "typos" in https://resources.whatwg.org/build/deploy.sh.

Ah, good catch. Thanks!

annevk commented 1 year ago

Looking at this again I'm not sure I understand the setup around name.

I pushed a cleanup commit that removes some of the wording around name that seemed to duplicate other requirements. And where it was used incorrectly as a type. I think we can go further, but I'm not entirely sure how to reword the various children iterators.

a-sully commented 1 year ago

I pushed a cleanup commit that removes some of the wording around name that seemed to duplicate other requirements. And where it was used incorrectly as a type

This looks good, assuming we agree below on how to handle the OPFS root. Thanks!

Looking at this again I'm not sure I understand the setup around name.

  • Do entries need a name at all?

    I'm not entirely sure how to reword the various children iterators.

Are we sure we need to reword the various children iterators? It seems fine to use entries (as opposed to locators) for file system operations like reading the contents of an entry (i.e. the bytes of a file, or the children of a directory).

We need some way to identify the children within a directory. Iterating through a directory doesn't make much sense if the files within it don't have a name...

If the intention is that an "entry" represents an OS-level file/directory, then giving each entry a name (and contents, as mentioned above) is probably what we want. Alternatively, we could consider giving each entry a path... but that's not really how most file systems work. You can later get a file's path by walking up the directory chain, but the path isn't inherent to the file itself.

  • Why does getDirectory() need to set a name? Can it not just set path to "« the empty string »"? That would also allow us to simplify the getter steps as they would not have to special case OPFS anymore.

Yeah setting the path to "« the empty string »" is what I'd proposed earlier, so I'm on board with that :) https://github.com/whatwg/fs/pull/96#discussion_r1125034492

The entry itself still needs a name though, as per above. And we need |map|["root"] to point to a directory entry that exists, not a locator that may not. Right?

annevk commented 1 year ago

Okay, that sounds reasonable. I guess that means we should keep the path[last]/name invariant in some form, probably as new bullet points in the lists I created. If you'll indulge me, what happens when you hold an entry named x in JS and someone does mv x y on the command line?

a-sully commented 1 year ago

Okay, that sounds reasonable. I guess that means we should keep the path[last]/name invariant in some form, probably as new bullet points in the lists I created.

SGTM. I'll add this in the next patch

If you'll indulge me, what happens when you hold an entry named x in JS and someone does mv x y on the command line?

So, as of this PR, JS (i.e. a FileSystemHandle) can't really "hold an entry", per se. If you have a FileSystemFileHandle whose locator pointed to x, then "locating an entry" with that locator will now return null. The entry itself would be renamed to y.

This isn't a huge deal, especially within the OPFS, since we don't expect users to be mving files within their browser's profile. It's a much bigger deal outside of the OPFS, where the files are visible to the user and to other applications on the machine. This is where https://github.com/WICG/file-system-access/issues/72 comes into play - if a file is moved, the web site might receive a change event containing a FileSystemFileHandle pointing to where the file was moved to. Initially, I was planning to only bother specifying this for non-OPFS files, since that's where the most obvious use cases are... but given the developer demand for things like https://github.com/w3c/IndexedDB/issues/51, I suspect it could provide some value to the OPFS, as well. Anyways, that's a discussion for another day. I'm hoping to put an explainer up soon :)