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

Native File System API #390

Closed
3 of 5 tasks
mkruisselbrink opened this issue Jun 24, 2019 · 21 comments
Closed
3 of 5 tasks

Native File System API #390

mkruisselbrink opened this issue Jun 24, 2019 · 21 comments
Assignees
Labels
Progress: review complete Provenance: Fugu Resolution: satisfied The TAG is satisfied with this design Review type: later review Topic: permissions Topic: powerful APIs APIs that reach into your life. Topic: Streams Any kind of stream-like feature Topic: universal JavaScript Features that work on the web and non-web (e.g. node.js) Venue: WICG

Comments

@mkruisselbrink
Copy link

こんにちはTAG!

I'm requesting a TAG review of:

Further details:

You should also know that...

As mentioned above, we fully expect to be iterating on the best shape of this API for at least the duration of the origin trial (i.e. most of 2019). We have some idea of what we want the API to look like, and what use cases we want to support, but also expect to learn from the origin trial that perhaps we were wrong about what is needed for which use cases.

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our GitHub repo for each point of feedback
  • open a single issue in our GitHub repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@kenchris kenchris self-assigned this Jun 24, 2019
@kenchris kenchris added Topic: permissions Topic: powerful APIs APIs that reach into your life. Topic: Streams Any kind of stream-like feature Topic: universal JavaScript Features that work on the web and non-web (e.g. node.js) Venue: WICG labels Jun 25, 2019
@kenchris
Copy link

I see some new API ideas here: WICG/file-system-access#19 (comment)

What is the status of that, will that be changed in the spec or not? because that affects review.

Also, I find inPlace confusing. It doesn't explain what inPlace does, and repeats the description of keepExistingData's behavior. Our understanding so far is that inPlace = true means it will write directly to the destination file without creating a temporary copy first, but it's not clear. So potentially faster, but I though the copy was there because of security and integrity - I understood you never wanted to write incomplete data.

@lknik
Copy link
Member

lknik commented Jul 22, 2019

In the security/privacy questionnaire, what do you mean by state as in in "The drive-by web will only have enough state to allow it to re-prompt for access, but the access itself won't be persistent.?

On "No, unless a device exposes such sensors as files or directories. User agents are encouraged to block access to such files or directories (for example /dev on linux like systems).", where in the specification UAs are encouraged to block the access? I think this is 5.1 but not stated directly?

I also see that in PWA mode this API will behave differently. There are not many such APIs as of now but I believe it would be useful to have the differences (permissions model) documented in the spec.

@kenchris
Copy link

Hi there,

We have been told that the feature will be rolled out in pieces, we would very much like to know the order of this roll out so that we can prioritize the review of this feature. Thanks!

@mkruisselbrink
Copy link
Author

We've tried to answer that question in the document linked to from the original review request, as well as in a reply to the question you raised about that in the spec repo.

Anyway, to summarize all that:

In initial Origin Trial (chrome 78), what is currently in the spec with the exception of:

  • FileSystemDirectoryHandle.resolve()
  • FileSystemWriter.asWritableStream() (or some other way of supporting writable streams)
  • Handles won't be Serializable
  • FileSystemCreateWriterOptions.inPlace. I.e. all writers will not be in-place writers (or phrased otherwise, all FileSystemWriter instances will have their atomic flag set to true.

Less clear in what order we'll work on these features afterwards, but probably streams and serializabilty will come before inPlace.

@alice alice removed this from the 2019-09-10-f2f-tokyo milestone Jan 27, 2020
@mkruisselbrink
Copy link
Author

Sorry for the delays here, our current plan is to do more work on the spec side of things/make sure it's up to date with what we're implementing over the next couple of weeks, so by the end of February we should have something ready that we'd like a review of.

@torgo
Copy link
Member

torgo commented Feb 10, 2020

Just noting that in the MDN Web Developer Needs Assessment survey results, "file system" access was reported as one of the key things developers felt was missing from the web. https://insights.developer.mozilla.org/

@cynthia
Copy link
Member

cynthia commented Mar 3, 2020

@kenchris and I discussed this during the Wellington F2F. So far, it looks like not much has changes since we last looked at it. (on the Github repo)

so by the end of February we should have something ready that we'd like a review of.

Is there a work-in-progress we can look at? Have there been any significant changes in the design?

@mkruisselbrink
Copy link
Author

I finally have been able to make some progress here, once WICG/file-system-access#168 and WICG/file-system-access#169 land there should be some normative spec text for everything in the spec at least (and the integration with writable streams has landed in the spec as well). Having said that, there are still plenty of known open issues and open questions.

@mkruisselbrink
Copy link
Author

Some particular questions I would love your input on:

  • different methods vs multiple options for one method (and different return types depending on what options were passed in), Should choose always return an array? WICG/file-system-access#25. Currently we have one method with a bunch of options, and depending on the options passed in you get a different return type (files vs directories, single handle vs sequence of handles). It might be nicer to have separate methods with return types that don't depend on the options, but having it all be one method doesn't seem crazy to me either.

  • And Symbol.asyncIterator on FileSystemDirectoryHandle? WICG/file-system-access#158, which is basically the question what the best way to integrate with async iterators would be: should a directory handle itself be (async) iterable with its "values" being the files/directories it contains, or should we keep the current API where there is a method that returns an async iterable. I'm leaning towards the first, but it feels a bit awkward to me to have a "values" getter rather than something more appropriately named (I suppose we could have both as well, but that might be the worst of both?)

So definitely would welcome your feedback on those issues (and anything else of course).

I hope to address some more of the earlier filed feedback and outstanding issues in the next week or two as well, again sorry for the delays.

@kenchris
Copy link

Generally, the return type should not differ depending on what arguments a method takes or what a user selected. Multiple and single file can be solved by always returning an array and letting that array just have the size of 1 in the case of a single file. I believe the existing input type=file support works that way

@cynthia
Copy link
Member

cynthia commented May 27, 2020

We've provided feedback on the first question here: WICG/file-system-access#25

It looks like the second question has already been addressed on your end.

@cynthia cynthia removed the Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review label May 27, 2020
@mkruisselbrink
Copy link
Author

Yes, thank you. I think the spec is getting pretty close to being something I'm happy with, and we're gearing up for shipping what is currently in the spec (or something very close to that) in hopefully Chrome 86. We've made a number of API changes in response to feedback we received (documented in https://github.com/WICG/native-file-system/blob/master/changes.md), and also resolved a number of spec issues. One more spec-internal, but large change is that we changed how permissions are specified to integrate with the permissions API (in WICG/file-system-access#200).

I would like you're input on WICG/file-system-access#210. Currently the spec defines a number of methods on the global, but another option would be to define those in their own FileSystem namespace. Not sure what the trade-off is between the two options. https://w3ctag.github.io/design-principles/#example-09c8f087 mentions to use namespaces instead of singletons, but doesn't really elaborate on just having new methods on the global vs having those new methods in a namespace. Any guidance you can give here?

I'd also like to point your attention at WICG/file-system-access#192. Our explainer used to say that we weren't going to integrate with drag&drop initially, but we've been having an intern work at that, so we might be including that in the initial version of the API we ship after all.

Other than that, we have a number of spec issues we're still working on resolving, but since these are unlikely to result in breaking API changes we haven't prioritized them so far.

And after shipping we are likely going to keep working on adding new features/methods to the API.

@cynthia
Copy link
Member

cynthia commented Sep 23, 2020

@kenchris and I discussed this during our "Cork" F2F.

The changelog was immensely helpful! Maybe we should make this a recommended practice. Thanks a lot for taking the time to write that.

Apologies that our feedback took so long. This is moving forward fast and it looks like our feedback wasn't timely - but we retroactively looked at WICG/file-system-access#210, and we don't have a strong opinion on whether this should be namespaced or in global. Since the amount of APIs being added is moderately low, it does seem fine to be on global - unless you have plans to add a lot more related to this capability in the future. (Although we don't quite see what is missing, so it would be good to know if you do have such plans.)

We also looked at the D&D integration, and it looks good to us. Good to see this being added, as we can see this improving user experience.

Since this seems to be in good shape, we are happy to consider this review finished, and would love to see the developers start using this. (We did see some misleading press about this, presumably written by someone who hasn't fully read the spec.) One last point - it looks like the work has reached a point where it needs a home (meaning, a working group) - is that something that is being discussed?

@cynthia cynthia added Progress: propose closing we think it should be closed but are waiting on some feedback or consensus Progress: review complete Resolution: satisfied The TAG is satisfied with this design Review type: later review and removed Progress: propose closing we think it should be closed but are waiting on some feedback or consensus labels Sep 23, 2020
@cynthia
Copy link
Member

cynthia commented Sep 23, 2020

Group consensus is that this is good to close. Thanks for incorporating the feedback!

@LeaVerou
Copy link
Member

Hi there, I know this is closed, but just a quick question wrt API design that I didn't see being brought up during the actual review: Why is this a set of global functions, instead of keyed off some kind of namespace?

@mkruisselbrink
Copy link
Author

Hi there, I know this is closed, but just a quick question wrt API design that I didn't see being brought up during the actual review: Why is this a set of global functions, instead of keyed off some kind of namespace?

That was raised by me in #390 (comment), and answered by a member of the Tag in the following comment.

@LeaVerou
Copy link
Member

Hi there, I know this is closed, but just a quick question wrt API design that I didn't see being brought up during the actual review: Why is this a set of global functions, instead of keyed off some kind of namespace?

That was raised by me in #390 (comment), and answered by a member of the Tag in the following comment.

This was before my time, but IMO there should have been stronger guidance against this. It's not just about namespacing, I think it's more confusing for authors when functions are just added to the global scope like this, as there's nothing tying related parts of an API together. I guess this ship has sailed now, but I'll open a design principles issue so we can avoid it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: review complete Provenance: Fugu Resolution: satisfied The TAG is satisfied with this design Review type: later review Topic: permissions Topic: powerful APIs APIs that reach into your life. Topic: Streams Any kind of stream-like feature Topic: universal JavaScript Features that work on the web and non-web (e.g. node.js) Venue: WICG
Projects
None yet
Development

No branches or pull requests

10 participants