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

#129 Draft - support for new fileHierarchy() function which adds a ha… #132

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

reubenfirmin
Copy link

…ndle to the requested directory as well as the content of that directory.

(Not verified yet - submitted for code review.)

… which adds a handle to the requested directory as well as the content of that directory
… which adds a handle to the requested directory as well as the content of that directory
@tomayac
Copy link
Member

tomayac commented Nov 24, 2022

I know this is still in flux.

If we have nested empty folders, they should be listed, too. So for example, for a file hierarchy like this:

/
├─ empty/
├─ full/
   ├─ file.txt

@tomayac
Copy link
Member

tomayac commented Nov 24, 2022

Currently your code would ignore the empty/ folder, since there's no file in it.

@reubenfirmin
Copy link
Author

reubenfirmin commented Nov 25, 2022

@tomayac Ah, that's interesting. The return type of directoryOpen, which is delegated to, is:

export interface FileWithDirectoryAndFileHandle extends File {
  directoryHandle?: FileSystemDirectoryHandle;
  handle?: FileSystemFileHandle;
}

It looks like what we would want it to return instead is something like:

FileWithDirectoryAndFileHandle | DirectoryWithParentAndDirectoryHandle

Given that this would be a breaking change for existing users, looks like I can no longer delegate to directoryOpen, and instead need to copy/paste and adjust for this function's need, agree?

One question before working on the next round: I am not clear what the legacySetup portion is doing in the typescript definition; and, the linked to issue has been resolved in the meantime. Safe enough to remove this in the fileHierarchy version of this function?

Thanks

@tomayac
Copy link
Member

tomayac commented Nov 25, 2022

@tomayac Ah, that's interesting. The return type of directoryOpen, which is delegated to, is:

export interface FileWithDirectoryAndFileHandle extends File {
  directoryHandle?: FileSystemDirectoryHandle;
  handle?: FileSystemFileHandle;
}

It looks like what we would want it to return instead is something like:

FileWithDirectoryAndFileHandle | DirectoryWithParentAndDirectoryHandle

Given that this would be a breaking change for existing users, looks like I can no longer delegate to directoryOpen, and instead need to copy/paste and adjust for this function's need, agree?

This would definitely need to be a new function, the existing function cannot be used, since it works entirely differently.

One question before working on the next round: I am not clear what the legacySetup portion is doing in the typescript definition; and, the linked to issue has been resolved in the meantime. Safe enough to remove this in the fileHierarchy version of this function?

Luckily the legacy setup would not be something you'd have to deal with in this case, since you'd only deal with the modern API. The reason the legacy setup exists is that when you cancel an <input type=file> dialog, no cancelation event is being sent, so the code needs to guess when such a cancelation event has happened.

Thanks

@tomayac
Copy link
Member

tomayac commented Jan 16, 2023

If you're still working on this PR, the function at https://github.com/tomayac/opfs-explorer/blob/f86be0405bef6c68532be3a9fd347eab89e511a1/contentscript.js#L5-L48 may be 95% of what you're looking for. If you're interested in porting it here, please proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants