w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
332 stars 56 forks source link

Native File System API #390

Closed mkruisselbrink closed 4 years ago

mkruisselbrink commented 5 years ago

こんにちは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):

kenchris commented 5 years ago

I see some new API ideas here: https://github.com/WICG/native-file-system/issues/19#issuecomment-490579093

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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:

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

cynthia commented 5 years ago

If the rollout is going to happen in multiple stages, it would be extremely helpful for the end users (developers) to get a spec version that only has the parts actually implemented. We see this pattern quite often nowadays, and think it's a point that the platform needs to improve on.

The subset of functionality shipping seems fine to me. (I can't speak for the rest of the TAG here, as this is one of those complicated topics where there are multiple opinions)

foolip commented 4 years ago

I did a review of the spec today and found this linked in an issue, so I thought I'd summarize what seem like the most important existing or new issues to me.

On API shape:

On behavior/interoperability:

Finally, cloud drive considerations are probably not a big deal, but could affect the shape/behavior of the API given feedback from experts on that.

mkruisselbrink commented 4 years ago

Thank you for the thorough review @foolip. I plan to spend most of next week working on the spec, so should have updates in a week or two.

kenchris commented 4 years ago

A lot of good work seems to be going on here and I also talking F2F with @oyiptong at a few occasions.

As the shipping of this feature will happen in parts, please advice us when you have something ready that you would like review of, so that we can concentrate on those pieces first.

@lknik could you look at this in details from security point of view?

mkruisselbrink commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

I finally have been able to make some progress here, once wicg/native-file-system#168 and wicg/native-file-system#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 commented 4 years ago

Some particular questions I would love your input on:

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 commented 4 years ago

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 commented 4 years ago

We've provided feedback on the first question here: https://github.com/WICG/native-file-system/issues/25

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

mkruisselbrink commented 4 years ago

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/native-file-system#200).

I would like you're input on WICG/native-file-system#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/native-file-system#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 commented 4 years ago

@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 commented 4 years ago

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

LeaVerou commented 1 year ago

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 commented 1 year ago

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 https://github.com/w3ctag/design-reviews/issues/390#issuecomment-668755957, and answered by a member of the Tag in the following comment.

LeaVerou commented 1 year ago

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.