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

New Modes of Creating a FileSystemWritableFileStream #148

Open
a-sully opened this issue Sep 20, 2023 · 3 comments
Open

New Modes of Creating a FileSystemWritableFileStream #148

a-sully opened this issue Sep 20, 2023 · 3 comments
Labels
addition/proposal New features or enhancements

Comments

@a-sully
Copy link
Collaborator

a-sully commented Sep 20, 2023

The proposal for a new locking scheme - intended primarily to address #34 - also proposes new modes of creating a writable file stream

Migrating the following conversation over from mozilla/standards-positions#861 (cc @jesup)

@a-sully:

One thing I think we should align on it the behavior of the "exclusive" mode. The explainer currently proposes the following:

enum FileSystemWritableFileStreamMode { 
  "exclusive", // Only one writer can exist at a time
  "siloed",    // Each writer created will have its own swap file
};

where "exclusive" was expected to still follow the current pattern of writing to a swap file (apologies that this was not explicitly called out in the explainer, which was primarily focused on SyncAccessHandles). It seems prudent to leave the door open to add a third option for exclusive + in-place writes (even if this mode would not be supported when writing to the local file system - WICG/file-system-access#260 has lots of discussion about this). For example:

enum FileSystemWritableFileStreamMode { 
  "exclusive-in-place", // Only one writer can exist at a time, which writes directly to the target file
  "exclusive-atomic",   // Only one writer can exist at a time, which writes to a swap file
  "siloed",             // Each writer created will have its own swap file; last writer to close() wins
};

Thoughts? Happy to move this to a spec issue to further discuss if needed!

@janvarga:

Only SyncAccessHandle can currently directly access the file. Some developers don't like sync APIs, so providing "exclusive-in-place" access via FileSystemWritableFileStream might look compelling from this point of view. "Abort" could only close the stream in that case indeed.

@a-sully a-sully added the addition/proposal New features or enhancements label Sep 20, 2023
@a-sully
Copy link
Collaborator Author

a-sully commented Sep 20, 2023

Only SyncAccessHandle can currently directly access the file. Some developers don't like sync APIs, so providing "exclusive-in-place" access via FileSystemWritableFileStream might look compelling from this point of view. "Abort" could only close the stream in that case indeed.

Right, "abort" would be a bit of a misnomer. But I think that's okay?

Another potential source of confusion might be that createWritable({ mode: "exclusive-in-place" }) would clear the file, since keepExistingData defaults to false. So most(?) developers using this presumably would want createWritable({ mode: "exclusive-in-place", keepExistingData: true })... but I think that's okay, too? Changing the default only for this mode seems even more likely to lead to confusion

@a-sully
Copy link
Collaborator Author

a-sully commented Sep 20, 2023

Also, curious to hear thoughts on this issue from @annevk or @szewai

@janvarga
Copy link

Right, "abort" would be a bit of a misnomer. But I think that's okay?

Yes, I think that's ok, just that the spec should probably mention that.

Another potential source of confusion might be that createWritable({ mode: "exclusive-in-place" }) would clear the file, since keepExistingData defaults to false. So most(?) developers using this presumably would want createWritable({ mode: "exclusive-in-place", keepExistingData: true })... but I think that's okay, too? Changing the default only for this mode seems even more likely to lead to confusion

Yeah, they would have to add "keepExistingData: true" too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements
Development

No branches or pull requests

2 participants