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

Make explicit that FileSystemHandle entries map to paths #30

Closed
wants to merge 1 commit into from

Conversation

a-sully
Copy link
Collaborator

@a-sully a-sully commented May 24, 2022

Be more explicit in describing how FileSystemHandles map to a path on disk, and the implications of this choice. Consider the following cases:

Clearing site data

// Create file at /dir/file.txt.
const dir_entry = root.getDirectoryHandle('dir', { create: true });
const file_entry = dir_entry.getFileHandle('file.txt', { create: true });

// ======== Clear site data, clearing all data in the OPFS. ========

// file_entry still points to /dir/file.txt. The promise rejects with NotFoundError.
await dir_entry.getFile();

// Re-create the directory in the fresh OPFS.
const dir_entry = root.getDirectoryHandle('dir', { create: true });

// file_entry may be able to write /dir/file.txt, despite the file not being explicitly created
// Whether this works may depend on the user agent's implementation?
await file_entry.createWritable();

Moving a file handle (see #10)

// Create file at /old/file.txt.
const dir_entry = root.getDirectoryHandle('old', { create: true });
const file_entry = dir_entry.getFileHandle('file.txt', { create: true });

// The file previously pointed to file at /new/file.txt now resides at /old/file.txt.
await dir_entry.move('new');

// file_entry still points to /old/file.txt. The promise rejects with NotFoundError.
await dir_entry.getFile();

// Restore the directory name. file_entry once again points to a valid path.
await dir_entry.move('old');

// file_entry still points to /old/file.txt. This call should succeed.
await file_entry.getFile();

Removing a file handle (see #9)

// Create file at /old/file.txt.
const dir_entry = root.getDirectoryHandle('old', { create: true });
const file_entry = dir_entry.getFileHandle('file.txt', { create: true });

// Remove the file.
await file_entry.remove();

// We can still write to a removed file.
await file_entry.createWritable();

Preview | Diff

@@ -111,8 +111,9 @@ is left up to individual user-agent implementations.
An [=/entry=] |a| is <dfn for="entry">the same as</dfn> an [=/entry=] |b| if |a| is equal to |b|, or
if |a| and |b| are backed by the same file or directory on the local file system.

Issue: TODO: Explain better how entries map to files on disk (multiple entries can map to the same file or
directory on disk but an entry doesn't have to map to any file on disk).
An [=/entry=] maps roughly to a path, but there is no guarantee this path exists on disk.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkruisselbrink do you have a suggestion for a better way to word this?

I'm still hedging with the use of "roughly" here. I'm not sure how to toe the line in being as explicit as we can about what exactly the entry is, while allowing for entries to point to places other than disk (Drive, etc)

@annevk
Copy link
Member

annevk commented May 24, 2022

So we already state that entries have a name, but it seems that isn't the full location. Can we give entries an additional field, called "path" or "location" that is a string or byte sequence or implementation-defined type representing a location on disk? And then we build the other things on top of that. I typically find that making the conceptual model explicit in terms of data structures makes it easy to build processing models on top.

@a-sully
Copy link
Collaborator Author

a-sully commented May 24, 2022

We've had a number of requests to expose the full path on the non-standardized portion of the API (WICG/file-system-access#178, for example), but for privacy reasons I don't see us ever exposing a path for handles outside of the OPFS.

That being said, I don't think a full location is something we'd want or need to expose within the OPFS, either. A relative path is already available by calling resolve() with the handle and navigator.storage.getDirectory() so I'm not sure if I see the value of providing this on the object itself?

There's also the question of how this path would interact with Buckets. For example, multiple handles could have the same path but still be distinct files because they're in different buckets (unless the path includes the bucket somehow, which is a can of worms I assume we don't want to open). The advantage of using resolve() is that you have to explicitly pass in what the handle is relative to.

On that note, I'm open to renaming resolve() to something like getRelativePath() (see #4)

@a-sully
Copy link
Collaborator Author

a-sully commented May 24, 2022

#14 is somewhat related

@annevk
Copy link
Member

annevk commented May 25, 2022

I'm saying that we add it as a (private) member to the "entry" concept. Not that we'd expose it through a public API.

@a-sully
Copy link
Collaborator Author

a-sully commented May 26, 2022

Ah I see. I did not realize we could spec a private member.

There's also the question of how this path would interact with Buckets ... unless the path includes the bucket somehow, which is a can of worms I assume we don't want to open

It sounds like this member SHOULD include the bucket information. Assuming every new bucket has a unique ID (which seems like something we could spec?), this would handle the "clear site data" case above (or future "clear bucket" mechanism), since it would guarantee that handles become invalidated (well, at least always return NotFoundError) alongside their respective buckets (since the path/location will never be re-created).

Should we prefer "location" to "path", since this generalizes better to things like in-memory file systems and makes it clear that this is not a path relative to a bucket, but a location identifier which should include bucket information?

Overall this seems reasonable to me as long as we're okay with the implications for move() and remove() mentioned above. @mkruisselbrink any other thoughts here?

@a-sully a-sully mentioned this pull request Sep 7, 2022
3 tasks
@a-sully a-sully closed this Feb 24, 2023
@a-sully a-sully deleted the clarify-filesystemhandle-path branch February 24, 2023 02:49
annevk pushed a commit that referenced this pull request Mar 22, 2023
A much more thorough (than #30) attempt at expressing that FileSystemHandle maps to a file path. At a high level, it proposes that:

* A FileSystemHandle has an associated "file system locator"
* A "file system locator" contains, among other things, a "file system path"
* A "file system entry" can be located given a "file system locator"
* You can get a "file system locator" from a "file system entry"

See #59 for the implications of this in practice.

Fixes #59.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants