react-dropzone / file-selector

Convert a DragEvent or file input to a list of File objects
MIT License
95 stars 33 forks source link

Introduce an option to ignore a File's absolute path in an Electron environment #36

Closed jmcrthrs closed 2 weeks ago

jmcrthrs commented 4 years ago

Do you want to request a feature or report a bug?

What is the current behavior?

When running in a browser environment, the native File object contains no path (a path is added by toFileWithPath) When running in an Electron environment, the native File object contains an absolute path.

https://www.electronjs.org/docs/api/file-object

Electron has added a path attribute to the File interface which exposes the file's real path on filesystem.

Because of this, file-selector skips adding a path to the File object: https://github.com/react-dropzone/file-selector/blob/1e23f600ade799c378cc0a65189708f68183cec8/src/file.ts#L20

What is the expected behavior?

As reported in https://github.com/react-dropzone/file-selector/issues/10, this behaviour is expected by some users. However I would like the option to ignore the absolute path, and instead use the relative path provided by toFileWithPath

A new boolean paramter could be provided to toFileWithPath that determines if a file's path property should be ignored.

This boolean paramter would be false by default to prevent breaking changes.

We would also have to provide a new option in react-dropzone: https://github.com/react-dropzone/react-dropzone/blob/1b1177daaa51d7cc542d59f32dfd1e2956b92a55/src/index.js#L68

If this is a feature request, what is motivation or use case for changing the behavior?

I would like consistency between Browser & Electron environments. I need this because I use the file's relative path to reconstruct the file's folder structure on our server.

Other info

rolandjitsu commented 4 years ago

The exposed method is fromEvent. So that method would need to have the option. But I'm just a little hesitant to add logic that needs to know about electron 🤔

jmcrthrs commented 4 years ago

Yes, any implementation should be as generic as possible, as it is possible in the future that the path may be available in other environments.

e.g. ignoreNativePath

Although maybe that name is misleading because the path returned bytoFileWithPath is considered native? e.g. if it comes from one of the Browser APIs.

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity in the past 60 days. It will be closed within 7 days if no further activity occurs. If the issue persists please comment here to bump your issue. Thank You - React Dropzone Maintaners

AlexanderArvidsson commented 2 years ago

I realize this is over a year old, but I need to bump this. We are converting our web application at our company into a desktop application and I ran into this problem.

Our uploading logic relies on the fact that we have relative paths in the input selector or the drag and drop of react-dropzone. By changing it to use absolute path in Electron, we have no way of getting a reliable folder structure based of the directory chosen or dropped.

Can we please get an option for this? I do not think this is only relevant to Electron, because other browsers or environments (such as neutralino.js perhaps) could also be providing a native absolute path. I could create a pull request for it, but we would need one for both react-dropzone and file-selector. I'd like to know if this is something you would even want to pursue, otherwise it would just be a waste of time for me.

@rolandjitsu You mentioned you are hesitant to add logic that needs to know about Electron, but that is exactly what you have in the current code. The code checks for an existing absolute path, but that introduces inconsistencies between environments because you have no idea what that path could be. There should definitely be an option to disable this, because it is not consistent with react-dropzone in the Web as you'd expect.

@jmcrthrs Did you figure out an alternative solution to this?

rolandjitsu commented 1 month ago

@AlexanderArvidsson I'm open to a PR if you're still up for it. I'd also need to try it myself to understand the behaviour.

AlexanderArvidsson commented 1 month ago

@AlexanderArvidsson I'm open to a PR if you're still up for it. I'd also need to try it myself to understand the behaviour.

We're no longer using this and instead wrote our own version of it since there weren't any options to disable that check.

rolandjitsu commented 1 month ago

@AlexanderArvidsson I'm open to a PR if you're still up for it. I'd also need to try it myself to understand the behaviour.

We're no longer using this and instead wrote our own version of it since there weren't any options to disable that check.

Understood. Sorry for the very late reply. Did you use a custom getFilesFromEvent (in react-dropzone) or did you re-implement the whole drag'n'drop functionality?

I still see value in supporting what's being asked.

AlexanderArvidsson commented 1 month ago

Understood. Sorry for the very late reply. Did you use a custom getFilesFromEvent (in react-dropzone) or did you re-implement the whole drag'n'drop functionality?

I still see value in supporting what's being asked.

No worries at all!

Yes, we used a custom getFilesFromEvent which was basically a copy paste (with license and attribution, of course) from file-selector with the necessary changes. We had to copy the entire file, because the other utility functions weren't exported. That's still being used today and works great across both Web, Electron and mobile.

rolandjitsu commented 1 month ago

Yes, we used a custom getFilesFromEvent which was basically a copy paste (with license and attribution, of course) from file-selector with the necessary changes. We had to copy the entire file, because the other utility functions weren't exported. That's still being used today and works great across both Web, Electron and mobile.

If you don't mind, do you think you could make a PR with that change so everyone can benefit from it? On the react-dropzone end we just need another prop that would say useNativePath or something like that (I can help with that part).

rolandjitsu commented 1 month ago

Actually, https://github.com/react-dropzone/file-selector/pull/70, may indirectly help. It provides a relativePath which is always set.

AlexanderArvidsson commented 1 month ago

Actually, #70, may indirectly help. It provides a relativePath which is always set.

Cool, if that gets merged I can give it a try and replace our own version, but I haven't looked into the PR in too much detail. I unfortunately don't have a lot of time right now.

rolandjitsu commented 1 month ago

Ok, no worries.

On Fri, Oct 4, 2024, 23:17 Alexander Arvidsson @.***> wrote:

Actually, #70 https://github.com/react-dropzone/file-selector/pull/70, may indirectly help. It provides a relativePath which is always set.

Cool, if that gets merged I can give it a try and replace our own version, but I haven't looked into the PR in too much detail. I unfortunately don't have a lot of time right now.

— Reply to this email directly, view it on GitHub https://github.com/react-dropzone/file-selector/issues/36#issuecomment-2394506446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALJIOPPBHPFLDHDCXIIMXTZZ3ZWXAVCNFSM6AAAAABPHPSRRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUGUYDMNBUGY . You are receiving this because you modified the open/close state.Message ID: @.***>

rolandjitsu commented 2 weeks ago

@AlexanderArvidsson note that there's now a {relativePath} property on the file which should address your request. If it's ok, I'm going to go ahead and close this issue.

jonkoops commented 2 weeks ago

Looks like this issue can be closed? I am not sure why it was re-opened.

rolandjitsu commented 2 weeks ago

@jmcrthrs could you provide some context as to why the issue was re-opened?

jmcrthrs commented 2 weeks ago

@jmcrthrs could you provide some context as to why the issue was re-opened?

That was unintentional. I have closed this issue.