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

fix: gracefully handle null items #517

Merged
merged 2 commits into from
Sep 21, 2023
Merged

Conversation

itwbaer
Copy link
Contributor

@itwbaer itwbaer commented Sep 11, 2023

Per the DataTransferItem API, if webkitGetAsEntry() is called on something that isn't a file or folder, null is returned. Sometimes when dragging and dropping a file, extra string meta data seems to be associated with the event (examples, dragging and dropping an image blob from a different browser tab, or dragging a message from the Outlook desktop app).
image

image

image

This causes an exception to be thrown, because the _traverseFileTree function assumes each item to be a valid InternalDataTransferItem. The end result is any valid file in the items array will not get uploaded after this exception occurs. If we can instead gracefully handle this case, valid directories will still get crawled and valid files will still get uploaded.

fix #514

@vercel
Copy link

vercel bot commented Sep 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
upload ❌ Failed (Inspect) Sep 20, 2023 8:10pm

@itwbaer itwbaer changed the title gracefully handle null files gracefully handle null items Sep 11, 2023
@itwbaer itwbaer changed the title gracefully handle null items fix: gracefully handle null items Sep 11, 2023
@itwbaer
Copy link
Contributor Author

itwbaer commented Sep 19, 2023

Hey @yoyo837, any chance I could get a review on this? This is my first contribution so let me know if I am missing anything or if you need more information. Thanks!

@yoyo837
Copy link
Member

yoyo837 commented Sep 19, 2023

Could you add some test case for it?

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #517 (415291a) into master (f78cca9) will increase coverage by 2.08%.
The diff coverage is 100.00%.

❗ Current head 415291a differs from pull request most recent head 415f466. Consider uploading reports for the commit 415f466 to get more accurate results

@@            Coverage Diff             @@
##           master     #517      +/-   ##
==========================================
+ Coverage   84.92%   87.00%   +2.08%     
==========================================
  Files           6        6              
  Lines         252      254       +2     
  Branches       63       62       -1     
==========================================
+ Hits          214      221       +7     
+ Misses         38       33       -5     
Files Changed Coverage Δ
src/traverseFileTree.ts 80.64% <100.00%> (+15.12%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@itwbaer
Copy link
Contributor Author

itwbaer commented Sep 20, 2023

@yoyo837 Added!

Copy link
Member

@yoyo837 yoyo837 left a comment

Choose a reason for hiding this comment

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

LGTM

@yoyo837 yoyo837 merged commit a729bcc into react-component:master Sep 21, 2023
5 checks passed
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.

Support directory AND file uploads via Drag-and-Drop
3 participants