-
Notifications
You must be signed in to change notification settings - Fork 65
Propose FileSystemSubscriptionManager #457
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
base: main
Are you sure you want to change the base?
Conversation
|
@domenic Could you please provide some feedback on the API shape? |
| FileSystemObserverObserverOptions options = {}); | ||
| // Unsubscribes to changes to a FileSystemHandle. Returns a Promise that | ||
| // resolves when the unsubscription completes. | ||
| Promise<void> unsubscribe(FileSystemHandle handle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is different from https://developer.mozilla.org/en-US/docs/Web/API/PushManager which puts the unsubscribe() on the subscription object, but the same as https://wicg.github.io/cookie-store/#CookieStoreManager .
I would be interested to know more about why these two APIs made different choices, and how that should guide our choices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some arguments here
- Adding
unsubscribe()toFileSystemHandleseems to suggest that a handle can be subscribed anywhere? But it actually only works in certain context (SW). - To align with FileSystemObserver.unobserve(handle). As stated in the background sction,
FileSysttemObserverinspires this proposal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, subscribe() doesn't return anything in our case. That makes sense.
|
Thanks for your speedy feedback! I've addressed most of comments. PTAL. |
| FileSystemObserverObserverOptions options = {}); | ||
| // Unsubscribes to changes to a FileSystemHandle. Returns a Promise that | ||
| // resolves when the unsubscription completes. | ||
| Promise<void> unsubscribe(FileSystemHandle handle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, subscribe() doesn't return anything in our case. That makes sense.
domenic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
|
Just curious, has this been discussed elsewhere, or is this PR the first time this is coming? To create some links, I will mention that this work seems to extend whatwg/fs#165. That PR, FileSystemObserver, should be a viable strategy towards accomplishing #72 and whatwg/fs#124, requests for observation/watching/file-notification. |
|
Hi @rektide, this PR does evolve from FileSystemObserver, which has been launched in Chromium. This new interface is trying to support such capability in service workers. It was only discussed internally in Chromium but not here. We are publishing this to look for more feedback before going into implementation. |
@fergald