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

Add proposal for the FileSystemObserver interface #124

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

a-sully
Copy link
Collaborator

@a-sully a-sully commented Jun 6, 2023

Proposal for a new interface which allows a website to be notified of changes to files and directories.

See #123 and WICG/file-system-access#72

@jespertheend
Copy link

Thanks for this proposal!
My application is currently not polling (for performance reasons) but traversing the file tree when the window receives focus.
A big use case for me is to make external changes to shader code and immediately see the results as you type. Which is currently not possible because you have to focus the browser window after every change.
So this proposal is certainly much appreciated!

I'm not a code owner so I might not be the right person for this. But these are some thoughts I had after reading the proposal.

  • When some browsers point to the same FileSystemHandle reference as one that already exists. Some developers might compare two handles using the equality operator rather than using isSameEntry(). They might then test their application in one browser, while another might not behave this way. To improve compatibility I think it would be good to explicitly create new FileSystemHandle instances for every FileSystemChangeRecord.
  • You mentioned that linux has no native support for recursive watches. Would this mean that changes to files in subdirectories are not reported at all? Or would it only mean that the FileSystemChangeType might not be as accurate?
  • Say a user agent supports the FileSystemObserver on some directories, but not on a specific directory.
    An application developer would need a way to know whether to fall back on polling or not. The "errored" FileSystemChangeType could work for this, but I'm worried that without explicitly mentioning what error should be thrown on the observe() call, that some browsers might end up with situations where simply no events will be fired at all.

@a-sully
Copy link
Collaborator Author

a-sully commented Jun 7, 2023

Thanks for this proposal! My application is currently not polling (for performance reasons) but traversing the file tree when the window receives focus. A big use case for me is to make external changes to shader code and immediately see the results as you type. Which is currently not possible because you have to focus the browser window after every change. So this proposal is certainly much appreciated!

Thanks for taking a look at the proposal! I expect it will address your use case :)

I'm not a code owner so I might not be the right person for this. But these are some thoughts I had after reading the proposal.

  • When some browsers point to the same FileSystemHandle reference as one that already exists. Some developers might compare two handles using the equality operator rather than using isSameEntry(). They might then test their application in one browser, while another might not behave this way. To improve compatibility I think it would be good to explicitly create new FileSystemHandle instances for every FileSystemChangeRecord.

Could you file a separate issue for this with a repro case? I did some basic tests and wasn't able to get == or === to return true for two handles pointing to the same file (i.e. where isSameEntry() returns true).

We should probably add web platform tests, especially if there is a discrepancy https://github.com/web-platform-tests/wpt/tree/master/fs

  • You mentioned that linux has no native support for recursive watches. Would this mean that changes to files in subdirectories are not reported at all? Or would it only mean that the FileSystemChangeType might not be as accurate?

The user agent will handle watching subdirectories. However, it is significantly more resource-intensive (i.e. consumes memory, and more importantly file descriptors) to recursively watch a directory using inotify, since a watch descriptor needs to be created for every subdirectory. This is an implementation detail that web authors ideally wouldn't need to worry about, but given the exponential difference in system resource use and that user agents will add restrictions on the number of (sub)directories that can be watched (and behavioral differences, such as that changes to a newly-mounted sub-directory could be missed while the watch descriptors are being created) it's unfortunately a detail that's hard to hide...

For example, on Linux it's much more likely that observer.observe(dirHandleWithManySubDirs, { recursive: true }) will fail (by hitting some limit set by either the user agent or OS) than observer.observe(dirHandleWithManySubDirs, { recursive: false })

  • Say a user agent supports the FileSystemObserver on some directories, but not on a specific directory.
    An application developer would need a way to know whether to fall back on polling or not. The "errored" FileSystemChangeType could work for this, but I'm worried that without explicitly mentioning what error should be thrown on the observe() call, that some browsers might end up with situations where simply no events will be fired at all.

Unfortunately, it's hard for the user agent to know much about why a watch has failed, either. Your best bet is to attempt to restart the observation by calling observe() again, which, if it fails, may throw a useful exception (e.g. a NotAllowedError if permissions were lost and you need to call requestPermission())

@a-sully a-sully requested a review from inexorabletash June 7, 2023 15:16
@inexorabletash inexorabletash merged commit 158693d into main Jun 7, 2023
@inexorabletash inexorabletash deleted the proposal-filesystemobserver branch June 7, 2023 22:08
@annevk
Copy link
Member

annevk commented Jun 8, 2023

As a reminder, changes like this shouldn't be committed without a Meta: prefix as per the commit message guidelines. It's not worth it to rewrite main for this, but please let's not do it again.

@jespertheend
Copy link

I did some basic tests and wasn't able to get == or === to return true for two handles

Ah no this was just a hypothetical situation in which a user agent might reuse existing references in FileSystemChangeRecords. But you're right, web platform tests would probably take care of that.

The user agent will handle watching subdirectories. However, it is significantly more resource-intensive (i.e. consumes memory, and more importantly file descriptors) to recursively watch a directory using inotify, since a watch descriptor needs to be created for every subdirectory. This is an implementation detail that web authors ideally wouldn't need to worry about, but given the exponential difference in system resource use and that user agents will add restrictions on the number of (sub)directories that can be watched (and behavioral differences, such as that changes to a newly-mounted sub-directory could be missed while the watch descriptors are being created) it's unfortunately a detail that's hard to hide...

Thanks for the clarification, that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements
Development

Successfully merging this pull request may close these issues.

4 participants