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

[BUG] the version 1.2 introduced a regression #106

Closed
buenjybar opened this issue Nov 7, 2024 · 17 comments
Closed

[BUG] the version 1.2 introduced a regression #106

buenjybar opened this issue Nov 7, 2024 · 17 comments

Comments

@buenjybar
Copy link

buenjybar commented Nov 7, 2024

Describe the bug
A clear and concise description of what the bug is.

When running the new version of react-dropzone: @14.3.5 link, I noticed a regression introduced in this library

🐛 This bug was introduced in this commit

The actual error is FileDrop-ky6DIxvn.js:5

TypeError: Cannot read properties of undefined (reading 'getFile')
    at FileDrop-ky6DIxvn.js:1:43484
    at Generator.next (<anonymous>)
    at FileDrop-ky6DIxvn.js:1:386
    at new Promise (<anonymous>)
    at E (FileDrop-ky6DIxvn.js:1:183)
    at FileDrop-ky6DIxvn.js:1:43435
    at async Promise.all (__/index 0)

Accordingly to the experimental documentation of DataTransferItem: getAsFileSystemHandle() method MDN

A Promise.
If the item's kind property is "file", and this item is accessed in the dragstart or drop event handlers, then the returned promise is fulfilled with a FileSystemFileHandle if the dragged item is a file or a FileSystemDirectoryHandle if the dragged item is a directory.
Otherwise, the promise fulfills with null.

The return value can be a promise or null, but the current code doesn't manage null values

Expected behavior
A clear and concise description of what you expected to happen.
This shouldn't crash at runtime

Desktop (please complete the following information):

  • userAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36

Additional context
Add any other context about the problem here.

It seems that the old/legacy behavior is working as expected.

@buenjybar buenjybar added the bug Something isn't working label Nov 7, 2024
@jonkoops
Copy link
Collaborator

jonkoops commented Nov 7, 2024

@rolandjitsu should we set up a v1 branch so we can fix this one without creating a conflict with the beta branch?

@rolandjitsu
Copy link
Collaborator

rolandjitsu commented Nov 7, 2024

That's a bug alright. Thanks @buenjybar for reporting it. @jonkoops yes, we'll need a 1.2.x branch where we cherry pick a commit from master after we make a PR to master with the same fix for v2.

Let me setup the 1.2.x branch.

@rolandjitsu
Copy link
Collaborator

Once #107 goes through, we'll make a bump in react-dropzone as well.

@jonkoops
Copy link
Collaborator

jonkoops commented Nov 8, 2024

@buenjybar can you provide some details on how this can be reproduced? Which browser is being used, and in what way is the library called? Please provide a minimal code example like we have in the README.

@buenjybar
Copy link
Author

Sure the scenario is being done in a cypress e2e tests, I will try to find the best way to reproduce this bug

@jonkoops
Copy link
Collaborator

jonkoops commented Nov 8, 2024

Great thanks! Please make sure to provide a minimal and reproducible example that does not include any third party libraries/frameworks such as Cypress, React, etc.

@buenjybar
Copy link
Author

you can see how to reproduce the bug here.
👉 https://github.com/buenjybar/react-dropzone-issue106/

@jonkoops
Copy link
Collaborator

jonkoops commented Nov 8, 2024

That example still contains a bunch of framework specific code, could you provide a minimal example without React and testing frameworks and only file-selector? Here is a StackBlitz project you can use as a base.

@buenjybar
Copy link
Author

That example still contains a bunch of framework specific code, could you provide a minimal example without React and testing frameworks and only file-selector? Here is a StackBlitz project you can use as a base.

In the code sample there is nothing sophisticated.
I have explained how to reproduce it in the README.md

$ npm install
$ npm run start
// in a different terminal
$ npm run cy:open

// on cypress
Click on "E2E Testing"
Run on Chrome v.130 (whatever)
Run the app.spec.cy.js
open the console and see the crash
TypeError: Cannot read properties of undefined (reading 'getFile')
    at FileDrop-ky6DIxvn.js:1:43484
    at Generator.next (<anonymous>)
    at FileDrop-ky6DIxvn.js:1:386
    at new Promise (<anonymous>)
    at E (FileDrop-ky6DIxvn.js:1:183)
    at FileDrop-ky6DIxvn.js:1:43435
    at async Promise.all (__/index 0)

buenjybar added a commit to buenjybar/file-selector that referenced this issue Nov 8, 2024
@buenjybar buenjybar mentioned this issue Nov 8, 2024
12 tasks
@buenjybar
Copy link
Author

I have added this to reproduce the issue #108

@jonkoops
Copy link
Collaborator

@buenjybar I did a bit of refactoring, could you tell me if #114 fixes the problem for you?

@rolandjitsu
Copy link
Collaborator

I've also filed cypress-io/cypress#30629 to see if the Cypress team can provide any insight.

@rolandjitsu
Copy link
Collaborator

rolandjitsu commented Nov 16, 2024

Hmmm ... it seems like this may be a browser issue. If you try the following in Chrome 130:

const dt = new document.defaultView.DataTransfer();
const js = JSON.stringify({test: 1});
const b = new Blob([js], {type: "application/json"});
const f = new File([b], "test.json");
dt.items[0].getAsFileSystemHandle().then(h => console.log(h))

You'll get undefined logged.

I'm not sure what that means, but that's where the problem is. We expect a file or dir handle, but we get undefined.

I mean, it is an experimental API, so it's to be expected. But in Chrome, the API should be available and work according to the compatibility table.

It might also be the fact that I've done that from the console and not the UI, which may be a security issue which gets blocked by the secure context feature? 😕

Copy link

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This issue has been resolved in version 3.0.0-beta.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@buenjybar
Copy link
Author

I can confirm the Fix in 2.1.2 👍
Thank you @rolandjitsu and @jonkoops

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

No branches or pull requests

3 participants