Skip to content

feat: add FileSystemFileHandle support #79

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

feat: add FileSystemFileHandle support #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1565024
feat: add FileSystemFileHandle support
tomayac Oct 4, 2024
5c3b17d
docs: Document getAsFileSystemHandle()
tomayac Oct 4, 2024
8defe93
Update src/file-selector.ts
tomayac Oct 7, 2024
f50c82c
Add the handle onto the returned file
tomayac Oct 7, 2024
c2a9fb2
Update src/file-selector.ts
tomayac Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Most browser support basic File selection with drag 'n' drop or file input:
* [File API](https://developer.mozilla.org/en-US/docs/Web/API/File#Browser_compatibility)
* [Drag Event](https://developer.mozilla.org/en-US/docs/Web/API/DragEvent#Browser_compatibility)
* [DataTransfer](https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer#Browser_compatibility)
* [DataTransferItem.getAsFileSystemHandle()](https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/getAsFileSystemHandle)
* [`<input type="file">`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#Browser_compatibility)

For folder drop we use the [FileSystem API](https://developer.mozilla.org/en-US/docs/Web/API/FileSystem) which has very limited support:
Expand Down
9 changes: 8 additions & 1 deletion src/file-selector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ function flatten<T>(items: any[]): T[] {
}

function fromDataTransferItem(item: DataTransferItem) {
if ('getAsFileSystemHandle' in DataTransferItem.prototype) {
Copy link
Collaborator

@rolandjitsu rolandjitsu Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some unit tests would be nice.

Copy link
Author

@tomayac tomayac Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how, it seems like this runs tests against jsDOM, which doesn't emulate this API, unless I'm misunderstanding?

Copy link
Collaborator

@rolandjitsu rolandjitsu Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you check, I've mocked some of the APIs in order to get some unit tests running.

Copy link
Author

@tomayac tomayac Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into it, but it appears to be a rabbit hole. The current tests fail with ReferenceError: DataTransferItem is not defined, which is documented as jsdom/jsdom#2913, and jsdom/jsdom#2913 (comment) suggests a polyfill, but installing it fails with unresolvable dependency issues. YOLO-updating all dependencies also doesn't work. I'm not familiar with Jest and don't have time to learn how to mock stuff with it. If someone else has idle cycles or experience, this would be appreciated. Else, feel free to close the PR if the tests are a required dependency. No hard feelings.

Copy link
Collaborator

@rolandjitsu rolandjitsu Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to have a look as well. As long as there are no regressions, we could probably merge without tests. Let me see what I can find.

I appreciate the contribution @tomayac

return (item as any).getAsFileSystemHandle().then((fileSystemHandle: any) => {
const file = fileSystemHandle.getFile();
file.handle = fileSystemHandle;
return file;
});
}
const file = item.getAsFile();
if (!file) {
return Promise.reject(`${item} is not a File`);
Expand Down Expand Up @@ -185,4 +192,4 @@ async function fromFileEntry(entry: any) {
// https://github.com/Microsoft/TypeScript/issues/3496#issuecomment-128553540
interface FileArray extends Array<FileValue> {}
type FileValue = FileWithPath
| FileArray[];
| FileArray[];
Loading