Skip to content

Make openFilePicker function return a promise with files - #58

Closed
MrKampla wants to merge 7 commits into
masterfrom
open-file-picker-promise
Closed

MrKampla wants to merge 7 commits into
masterfrom
open-file-picker-promise

Conversation

@MrKampla

Copy link
Copy Markdown
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
Copy Markdown

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

MrKampla commented Apr 4, 2023

Copy link
Copy Markdown
Collaborator Author

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