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

Clarify behavior of createWritable() when the corresponding file does not exist #125

Open
a-sully opened this issue Jun 6, 2023 · 1 comment
Labels
clarification Standard could be clearer needs tests Moving the issue forward requires someone to write tests

Comments

@a-sully
Copy link
Collaborator

a-sully commented Jun 6, 2023

As part of the changes for #96, the FileSystemFileHandle.createWritable() algorithm now specifies a NotFoundError promise rejection if the file corresponding to the FileSystemFileHandle does not exist. See https://fs.spec.whatwg.org/#dom-filesystemfilehandle-createwritable

Unfortunately, this does not match the behavior that has been implemented in Chromium browsers for a quite long time; specifically that

Which... is a bit of an odd state to be in anyways. It seems that this was a quirk of the Chromium implementation rather than an intentional choice of behavior. If we had a WPT covering the first case, this likely would have been noticed sooner

One quirk of this behavior is that, since there does not exist an explicit way to "create self" given a file handle, createWritable({ keepExistingData: false }) can be used to "create self" after a handle was removed (while no such workaround exists for a directory handle). This means that sites which relied on the keepExistingData: false behavior to re-create a file may break.

Notably, remove() self was only recently shipped on Chromium browsers (and the spec PR #9 has been blocked on unrelated changes for a while), so the chances of breakage is likely low.

Personally, I would prefer (and it was indicated in previous discussions that @szewai might prefer #59 (comment)) an explicit create() method (see #126) rather than relying on a quirk of createWritable() to perform "create self" on a file handle. Barring any objections, I think we should converge on "createWritable() will always reject with a NotFoundError if the underlying file does not exist", then add WPTs to assert this behavior and perhaps add a warning in the spec of the behavior change on Chromium browsers. Thoughts? (FYI @annevk @jesup @szewai)

@a-sully a-sully added clarification Standard could be clearer needs tests Moving the issue forward requires someone to write tests labels Jun 6, 2023
@annevk
Copy link
Member

annevk commented Jun 7, 2023

Sounds good, except I don't think we should add a warning in the specification. Ideally what we write can last a long long time. Gotchas are more the domain for MDN, CanIUse, and web-developer-facing articles.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 7, 2023
DO NOT MERGE (yet) whatwg/fs#125

Currently, the createWritable() algorithm specifies that we must throw
a NotFoundError if the file corresponding to the FileSystemHandle does
not exist. See
https://fs.spec.whatwg.org/#dom-filesystemfilehandle-createwritable

Unfortunately, this does not match the behavior that has been
implemented in Chrome for a very long time; specifically that
- createWritable({ keepExistingData: true }) fails if the file does
  not exist, since there is no existing data to copy to the swap file
- createWritable({ keepExistingData: false }) succeeds if the file
  does not exist, since there is no existing data to copy. It still
  fails if the parent directory does not exist, however

Bug: 1405851
Change-Id: I788c5b177c188862d4b08b5dd876404522fa32d5
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 29, 2023
DO NOT MERGE (yet) whatwg/fs#125

Currently, the createWritable() algorithm specifies that we must throw
a NotFoundError if the file corresponding to the FileSystemHandle does
not exist. See
https://fs.spec.whatwg.org/#dom-filesystemfilehandle-createwritable

Unfortunately, this does not match the behavior that has been
implemented in Chrome for a very long time; specifically that
- createWritable({ keepExistingData: true }) fails if the file does
  not exist, since there is no existing data to copy to the swap file
- createWritable({ keepExistingData: false }) succeeds if the file
  does not exist, since there is no existing data to copy. It still
  fails if the parent directory does not exist, however

Bug: 1405851
Change-Id: I788c5b177c188862d4b08b5dd876404522fa32d5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer needs tests Moving the issue forward requires someone to write tests
Development

No branches or pull requests

2 participants