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

Make openFilePicker function return a promise with files #58

Closed
wants to merge 7 commits into from

Conversation

MrKampla
Copy link
Collaborator

This is a PR containing requested changes mentioned in #56
After this update, the openFilePicker function returns a promise with plain files, files content and errors.

Other important changes from maintainer's point of view:

  • useEffect has been removed. OpenFilePicker is now promise based and sets the state only as a result of user actions. This fixes unnecessary rerenders.
  • migrated from tsdx to dts. TSDX is no longer maintained, and it was causing some problems. Maitaining the package should be easier from now on.
  • bump typescript to 4.9.5
  • bump use-file-picker to v1.6.0

@Jaaneek please check it out

@MrKampla MrKampla requested a review from Jaaneek March 30, 2023 23:59
@github-actions
Copy link

size-limit report 📦

Path Size
dist/use-file-picker.cjs.production.min.js 5.59 KB (+70.68% 🔺)
dist/use-file-picker.esm.js 5.59 KB (+49.5% 🔺)

@MrKampla MrKampla mentioned this pull request Apr 1, 2023
@MrKampla
Copy link
Collaborator Author

MrKampla commented Apr 4, 2023

After thorough examination within the maintainers team, we decided to drop this promise feature due to problems with resolving promise when user cancels selection. Browsers do not fire any meaningful events when a user cancels selection, so we weren't able to resolve or reject the promise in that scenario, leaving the promise in the "pending" state indefinitely. This is not congruent with the promise spec and expectations of users, and it adds unnecessary performance overhead.
We tested several solutions to this issue, but we weren't happy with any of them. We were able to get this working on chrome on PC, but none of the implementations worked on mobile (both iOS and android) satisfyingly.
Instead of that, we'll implement onChange handlers which are not guaranteed to run when user cancels selection, but it is ok because we shouldn't care about that event anyway (it doesn't change the state). OnChange handlers implementation will be added in the following PR: #60

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.

1 participant