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

FileSystemSyncAccessHandle is all synchronous, docs say it's sometimes async #28647

Closed
karyon opened this issue Aug 18, 2023 · 3 comments · Fixed by #28814 or mdn/browser-compat-data#20621
Labels
needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.

Comments

@karyon
Copy link

karyon commented Aug 18, 2023

MDN URL

https://developer.mozilla.org/en-US/docs/Web/API/FileSystemSyncAccessHandle

What specific section or headline is this issue about?

Examples, Browser compatibility

What information was incorrect, unhelpful, or incomplete?

This page and sub-pages on individual methods say

Note: In earlier versions of the spec, close(), flush(), getSize(), and truncate() were wrongly specified as asynchronous methods. This has now been amended, but some browsers still support the asynchronous versions.

Furthermore, the compatibility table says that on chromium-browsers and safari, the sync variant must be enabled/is not supported.

This is not correct. While the original PR that added these docs states "Chrome 108 has introduced new synchronous versions of these methods, behind a flag", after that PR was opened the chromium team decided to ship only the sync variants (feature status page, intent to ship discussion, release announcement).
Regarding Safari, we tested this and it seems to only ship the sync variants, but I did not find any public information on that.

What did you expect to see?

In my opinion, all mentions of the async variants can be removed. As far as I can tell, it does not exist in any browser anymore.

Do you have any supporting links, references, or citations?

No response

Do you have anything more you want to share?

I can have a try at a PR if I'm told what the desired solution is and if there's any coordination of the PRs needed given that browser-compat-data also needs to be changed.

MDN metadata

Page report details
@karyon karyon added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Aug 18, 2023
@github-actions github-actions bot added Content:WebAPI Web API docs and removed Content:WebAPI Web API docs labels Aug 18, 2023
signal-coder added a commit to signal-coder/content that referenced this issue Aug 19, 2023
@wbamberg
Copy link
Collaborator

Hi @karyon , thank you for filing!

Technically MDN's policy is to keep documentation for things for 2 years since they stopped being shipped in browsers. So if the async versions did ship in Chrome 102, and were not changed until 108, then we would need to keep some indication of this until the end of November 2024.

If that's the case then we would need to keep the notes, but as you say they seem to be out of date. Perhaps we could amend them like:

Note: In earlier versions of the spec, close(), flush(), getSize(), and truncate() were wrongly specified as asynchronous methods, and older versions of some browsers implement them in this way. However, all current browsers which support these methods implement them as synchronous methods.

...then for the browser compat, I might suggest inverting things, so instead of the subfeature being (for example) "Synchronous implementation of the close() method" , we make the subfeature describe the old way. So we have a subfeature like "Asynchronous implementation of the close() method", and mark it supported 102-108.

I'm not sure what the Safari situation is though? From https://webkit.org/blog/12257/the-file-system-access-api-with-origin-private-file-system/ it looks like the API landed in Safari while the methods were still specced async, and Chris's PR does indicate that they were async. Unfortunately I can't get the demos to work in Safari, so I wasn't able to check the current state.

But honestly, given that everyone seems to regard the async versions as a mistake, it would probably be OK just to document Safari as only supporting sync.

cc @chrisdavidmills .

@chrisdavidmills
Copy link
Contributor

chrisdavidmills commented Aug 29, 2023

@karyon @wbamberg I agree with the suggested fix. Thanks for bringing this to my attention — I'll file implementation PRs today.

@chrisdavidmills
Copy link
Contributor

As an FYI, I found my old test demo for this API and tested it: https://scintillating-internal-crowd.glitch.me

It works fine in Safari, Firefox, and Chrome, so I'm confident in the changes suggested here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened.
Projects
None yet
3 participants